-
Notifications
You must be signed in to change notification settings - Fork 58
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
Support CUDA 12.2 #328
Support CUDA 12.2 #328
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing cufile being pulled in arm 12.0 testing. I think we want to disable that, right? Note that it's pulling the 12.3 version of cufile (1.8.1.2) from the nvidia channel since it can't get it from cf.
I think I fixed this in 9aeab68, we'll need to check CI closely on this repo. |
The aarch64 / libcufile part needed a bit more work. I took a stronger approach in f45ac7b, which removes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to think through exactly what cases are OK here and which aren't. kvikio dlopens libcufile, so in principle we ought to be able to put the runtime protections in place to avoid trying to load libcufile even if it is installed into an environment where it is not supported (CUDA 12.0, aarch64). We don't have to solve that in this PR (we can merge 12.2 support without aarch64 libcufile support) but I do think we should be able to enable this in a follow-up without waiting to drop CUDA 12.0/12.1. We can just allow installation of libcufile but make sure the library isn't loaded at runtime.
If we're OK with that result, can we just open an issue to track that as future work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry meant to approve based on my previous comment.
Great. You're right, that sounds plausible. I hadn't considered that this could be a runtime guard because it's dlopen'd. #339 Merging this for now. |
/merge |
This PR closes #341. The kvikio `INTERFACE_COMPILE_DEFINITIONS` were being set based on the packages available during the libkvikio conda build (e.g. CUDA 12.2 since #328), which might not be the same packages/versions as when libkvikio is actually being used (e.g. to build libcudf with CUDA 12.0). Because we built libkvikio with CUDA 12.2 and then tried to use it with CUDA 12.0 devcontainers, the build failed to find the cuFile Stream APIs that were introduced in CUDA 12.2. This PR defers these definitions until the call to `find_package`, which will then use the exact cuFile features present (if cuFile is available at all) when building a package like cudf that depends on kvikio. The libkvikio example/test binary is built with the cuFile features available at build time, for use in the `libkvikio-tests` conda package. However, this test binary will still be compatible with a runtime where cuFile is unavailable or is version 12.0, as it is dlopen-ing the library and has runtime checks for the batch/stream features it tries to use. I did local testing of this PR with cudf devcontainers. I tested both 12.0 and 12.2 to reproduce (and fix) the failure, and also tested clean builds of libcudf after removing `libcufile` (to test when cuFile is not found). All seems to work as intended. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Robert Maynard (https://github.com/robertmaynard) - Vyas Ramasubramani (https://github.com/vyasr) URL: #342
Follow-up to #328 For all GitHub Actions configs, replaces uses of the `test-cuda-12.2` branch on `shared-workflows` with `branch-24.04`, now that rapidsai/shared-workflows#166 has been merged. ### Notes for Reviewers This is part of ongoing work to build and test packages against CUDA 12.2 across all of RAPIDS. For more details see: * rapidsai/build-planning#7 *(created with `rapids-reviser`)* Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Ray Douglass (https://github.com/raydouglass) URL: #343
Description
Notes for Reviewers
This is part of ongoing work to build and test packages against CUDA 12.2.2 across all of RAPIDS.
For more details see:
Planning a second round of PRs to revert these references back to a proper
branch-24.{nn}
release branch ofshared-workflows
once rapidsai/shared-workflows#166 is merged.(created with
rapids-reviser
)