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

update cuda cmake code #232

Closed
wants to merge 2 commits into from
Closed

update cuda cmake code #232

wants to merge 2 commits into from

Conversation

robertu94
Copy link

@robertu94 robertu94 commented Jun 27, 2024

Hi @lindstro, some students I am working with encountered problems building the CUDA version of ZFP using a new version of CMake (v3.27 and newer). This is because the FindCUDA cmake package was disabled in favor of FindCUDAToolkit and CUDA language support in this version and will be removed in a future version after being deprecated in 3.10. The replacement features were introduced in cmake 3.17, but I find they are stable after 3.20 (the new cmake minimum required version that I introduced here).

As I have documented here: https://robertu94.github.io/guides/dependencies.html#tooling-versions, cmake 3.20 is available on all major Linux distros for their normal support cycles except Ubuntu 20.04, and CMake 3.20 is available on this platforms via pip install cmake or other package management systems including spack. Supporting cmake 3.17 also includes CentOS-7 until it is deprecated in July, but will not support Ubuntu 20.04 which only provides cmake 3.16.

Regardless if you adopt this patch or not the spack package for ZFP needs to be updated as it does not build the cuda variant after cmake 3.27 to something of the effect of depends("cmake@:3.26", when="@:1.0.0+cuda")

cc: @jtian0

@lindstro
Copy link
Member

@robertu94 I appreciate the contribution and would like to merge your PR, though we first need to find a workaround for the AppVeyor tests, which use CMAKE 3.16. I've been wanting to transition off AppVeyor and move to GitHub Actions. I can't remember what the issues were when we last looked into this, and I'm afraid I don't have time to tackle that at the moment.

Another minor challenge is that we've been doing a lot of CUDA (and HIP) work on the staging branch (which has not been pushed to in a while). This branch consolidates work from multiple branches, including new variable-rate (de)compression work and HIP support. Once complete, we will merge all this new work into develop.

I would prefer to incorporate these changes there first and work out any potential kinks before merging into develop. Currently our only GPU tests are being run at LLNL through GitLab CI, and so we'll also need to see if there are any issues there before I'm ready to merge into develop.

If you don't mind, can you please retarget your PR to the staging branch? I expect it will be another couple of months or so before we will complete work on staging and will be ready to merge into develop.

jtian0 added a commit to jtian0/24_SC_artifacts that referenced this pull request Jun 28, 2024
- synchronous with LLNL/zfp#232
@robertu94 robertu94 changed the base branch from develop to staging July 1, 2024 14:12
@lindstro lindstro mentioned this pull request Jul 16, 2024
@lindstro
Copy link
Member

@robertu94 A quick update: This PR conflicts with other changes we've recently made on the staging branch. Some of your proposed changes have, however, been adopted in the latest commits on that branch, which also address #178. This essentially required bumping CMake to 3.23, but only when ZFP_WITH_CUDA is enabled--otherwise we stick with CMake 3.9. Please reopen if these changes do not address your needs.

@robertu94
Copy link
Author

Thanks @lindstro to close the loop, this fix works for me.

@robertu94 robertu94 closed this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants