Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Libxc] update to version 7.0.0 #9910

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+35 −123
Open

[Libxc] update to version 7.0.0 #9910

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4e3d494
[Libxc] update to version 7.0.0
abussy Oct 8, 2024
6d2d7ef
Clean-up
abussy Dec 3, 2024
33f77f7
cosmetics
abussy Dec 4, 2024
040da1f
add trailing coma back in
abussy Dec 9, 2024
b8326d8
Merge branch 'master' into pr/abussy/9910
imciner2 Dec 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions L/Libxc/Libxc/build_tarballs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using BinaryBuilder, Pkg

name = "Libxc"
version = v"6.1.0"
version = v"7.0.0"
include("../sources.jl")


Expand Down Expand Up @@ -37,4 +37,4 @@ dependencies = [

# Build the tarballs, and possibly a `build.jl` as well.
build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies;
preferred_gcc_version=v"5", julia_compat="1.6")
preferred_gcc_version=v"8", julia_compat="1.8")
38 changes: 15 additions & 23 deletions L/Libxc/Libxc_GPU/build_tarballs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,61 +5,53 @@ include(joinpath(YGGDRASIL_DIR, "fancy_toys.jl"))
include(joinpath(YGGDRASIL_DIR, "platforms", "cuda.jl"))

name = "Libxc_GPU"
version = v"6.1.0"
version = v"7.0.0"
include("../sources.jl")

sources = [
sources;
DirectorySource("./bundled")
]

# Bash recipe for building GPU version
# Notes:
# - 3rd and 4th derivatives (KXC, LXC) not built since gives a binary size of ~200MB
script = raw"""
cd $WORKSPACE/srcdir/libxc-*/

# Needed for Libxc 6.1.0 as these backport some fixes on libxc master
# On Libxc > 6.1.0 we can also remove the -DBUILD_TESTING=OFF
atomic_patch -p1 ${WORKSPACE}/srcdir/patches/cmake-cuda.patch
atomic_patch -p1 ${WORKSPACE}/srcdir/patches/source-fixes.patch
ln -s $prefix/cuda/lib $prefix/cuda/lib64

mkdir libxc_build
cd libxc_build
cmake -DCMAKE_INSTALL_PREFIX=$prefix -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN} \

cmake -DCMAKE_INSTALL_PREFIX=$prefix -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN}\
-DCMAKE_BUILD_TYPE=Release -DENABLE_XHOST=OFF -DBUILD_SHARED_LIBS=ON \
-DENABLE_CUDA=ON -DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc -DBUILD_TESTING=OFF \
-DENABLE_FORTRAN=OFF -DDISABLE_KXC=ON ..
-DENABLE_CUDA=ON -DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libxc is using enable_language(CUDA), i.e. the FindCUDAToolkit CMake module, so it should be sufficient (and a bit more proper) to specify the CUDA toolkit root, i.e. CUDAToolkit_ROOT=$prefix/cuda - instead of specifying the path to nvcc.

... but for unknown reasons, it seems CUDA_TOOLKIT_ROOT_DIR=$prefix/cuda is usually what actually works - though that is the variable specified for the deprecated FindCUDA

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the CUDA_TOOLKIT_ROOT_DIR or CUDAToolkit_ROOT option works out of the box in this build, without exporting the location of the nvcc compiler. I left the DCMAKE_CUDA_COMPILER as it is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try setting CUDA_PATH?

-DBUILD_TESTING=OFF -DENABLE_FORTRAN=OFF \
-DDISABLE_KXC=ON ..
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking lists into separate parts and sorting them is good practice (helps shorten future diffs etc.):

Suggested change
-DENABLE_CUDA=ON -DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc \
-DBUILD_TESTING=OFF -DENABLE_FORTRAN=OFF \
-DDISABLE_KXC=ON ..
-DBUILD_TESTING=OFF \
-DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc \
-DENABLE_CUDA=ON \
-DENABLE_FORTRAN=OFF \
-DDISABLE_KXC=ON ..

This should of course be extended for the entire cmake "configure" call, but GitHub prevents making a suggestion for the entire cmake configure call (due to some deleted lines, apparently).


make -j${nproc}
make install
"""
cmake --build . --parallel $nproc
cmake --install .

augment_platform_block = CUDA.augment
unlink $prefix/cuda/lib64
"""

# Override the default platforms
platforms = CUDA.supported_platforms()
platforms = CUDA.supported_platforms(; min_version=v"11.4")
filter!(p -> arch(p) == "x86_64", platforms)


# The products that we will ensure are always built
products = [
LibraryProduct("libxc", :libxc)
]

# Dependencies that must be installed before this package can be built
dependencies = [
Dependency(PackageSpec(name="CompilerSupportLibraries_jll", uuid="e66e0078-7015-5450-92f7-15fbd957f2ae")),
Dependency(PackageSpec(name="CompilerSupportLibraries_jll", uuid="e66e0078-7015-5450-92f7-15fbd957f2ae"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't make unrelated unneeded and undesired cosmetic changes such as removing trailing commas that are there for a very good reason (i.e. not to break git blame when adding new lines)

Suggested change
Dependency(PackageSpec(name="CompilerSupportLibraries_jll", uuid="e66e0078-7015-5450-92f7-15fbd957f2ae"))
Dependency(PackageSpec(name="CompilerSupportLibraries_jll", uuid="e66e0078-7015-5450-92f7-15fbd957f2ae")),

]

# Build Libxc for all supported CUDA toolkits
for platform in platforms
should_build_platform(triplet(platform)) || continue

cuda_deps = CUDA.required_dependencies(platform)
cuda_deps = CUDA.required_dependencies(platform; static_sdk=true)

build_tarballs(ARGS, name, version, sources, script, [platform],
products, [dependencies; cuda_deps]; lazy_artifacts=true,
julia_compat="1.7", augment_platform_block,
skip_audit=true, dont_dlopen=true)
julia_compat="1.8", augment_platform_block=CUDA.augment, preferred_gcc_version=v"8")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider sorting keyword args (also lazy_artifacts) and putting each on a separate line.

end
72 changes: 0 additions & 72 deletions L/Libxc/Libxc_GPU/bundled/patches/cmake-cuda.patch
View file
Open in desktop

This file was deleted.

21 changes: 0 additions & 21 deletions L/Libxc/Libxc_GPU/bundled/patches/source-fixes.patch
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion L/Libxc/sources.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Sources required for all builds
sources = [
ArchiveSource("https://gitlab.com/libxc/libxc/-/archive/$(version)/libxc-$(version).tar.gz",
"f593745fa47ebfb9ddc467aaafdc2fa1275f0d7250c692ce9761389a90dd8eaf"),
"8d4e343041c9cd869833822f57744872076ae709a613c118d70605539fb13a77"),
]