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.

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

[SuiteSparse_GPU] Update products to only contain GPU libraries #7803

Closed
wants to merge 3 commits into from

Conversation

imciner2
Copy link
Member

The registration for the v7 SuiteSparse GPU JLL failed because some SuiteSparse libraries (such as AMD) were declared as products in the recipe, but we weren't actually building them in the GPU JLL because we were going to be using the CPU-built ones in the normal JLL (since there isn't a GPU-specific version of those libraries.

Log file of the failure: https://buildkite.com/julialang/yggdrasil/builds/6438#018baa65-8284-4658-b64a-39baf25356f7, error:

ERROR: LoadError: Unable to locate LibraryProduct(["libamd"], :libamd, String[], false, Symbol[]) within /tmp/jl_C9Wk2N for x86_64-linux-gnu-cuda+11.4

Instead, we need to declare only the GPU-accelerated libraries in the GPU recipe.

@imciner2 imciner2 marked this pull request as draft December 14, 2023 13:45
@imciner2 imciner2 added cuda 🕹️ Builders related to Nvidia CUDA linear algebra 🔢 labels Dec 14, 2023
@imciner2
Copy link
Member Author

The 7.3.0 version we have doesn't play nicely with our CMake right now because it wants the cuda::NvToolsExt target, but that has been replaced with a cuda::nvtx3 target. This has been fixed in what will become the upstream 7.4.0 release, and there are quite a few major changes to the build system between 7.3.0 and 7.4.0 that makes it impossible to backport things to our 7.3.0 build easily. Since 7.4.0 has some beta releases right now, I am just going to hold off on finishing this until 7.4.0 is released and we rebase onto 7.4.0.

(note 7.4.0 also seems to simplify how we can do the CMake builds, so we shouldn't need the for loop over targets and instead just pass a list of targets to a root CMake instead).

@ViralBShah
Copy link
Member

@imciner2 Would you be able to update this to 7.4? Would be nice to get it merged.

@imciner2
Copy link
Member Author

imciner2 commented Jan 7, 2024

Yes, this is on my list to revisit later this week. I believe it is close to working and that I just need to rebase it.

@imciner2
Copy link
Member Author

Ok, this is now rebased to 7.4 fully, and it builds. I haven't tried any tests on it though, so I can't say if it actually works.

That said, apparently SuiteSparse 7.5 will be released today, so since this also touched the actual SuiteSparse build script to cleanup some commonalities, we could do that update in here and not have an extra build of the CPU libraries.

@ViralBShah
Copy link
Member

Yeah, makes sense to wait for 7.5. The first step to making all this work on the GPU is to have a working build. I hope to test it out once it gets merged.

@imciner2
Copy link
Member Author

imciner2 commented Jan 11, 2024

Ok, I put up the 7.5 update in a separate PR (#7931). Unfortunately, I don't think we can properly bump the GPU and non-GPU libraries in the same PR due to dependency issues. The GPU libraries will use the AMD compiled in the normal SuiteSparse_jll, and so they have a specific version dependency apparently (e.g. I just tested building the GPU version of 7.5.0 against the 7.4.0 jll in the general registry right now, and 7.5.0 wants AMD 3.3.1 minimum, whereas 7.4.0 has 3.3.0).

I think we might also need to add a strict version requirement to the SuiteSparse dependency in this package also - to pin the SuiteSparse and SuiteSparse_GPU packages to be the exact same SuiteSparse version.

@rayegun
Copy link
Contributor

rayegun commented Jan 11, 2024

They do need to be the exact same. SPQR_GPU and CHOLMOD_GPU are technically just part of their respective parent packages.

Same with SPQR <=> CHOLMOD. I would not trust trying to separate them even if it's possible for some versions by accident

@imciner2
Copy link
Member Author

They do need to be the exact same. SPQR_GPU and CHOLMOD_GPU are technically just part of their respective parent packages.

Great, thanks for confirming. I've added a strict version requirement on the updated PR (https://github.com/JuliaPackaging/Yggdrasil/pull/7932/files#diff-331b2e16e9da7ac54597cf58b1baae5feedef23d91ce85981be8fe645a15474cR99), so that should force the same version when installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda 🕹️ Builders related to Nvidia CUDA linear algebra 🔢
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants