Make use_cuda_wheels and cuda_suffixed explicit - #23472
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesCUDA wheel dependency alignment
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
use_cuda_wheels: "true" explicituse_cuda_wheels and cuda_suffixed explicit
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dependencies.yaml`:
- Line 206: Update the libcudf dependency matrix entry identified by
cuda_suffixed so it matches python/libcudf/pyproject.toml’s
cuda_suffixed=true;use_cuda_wheels=true contract, or regenerate that pyproject
metadata from the corrected matrix. Ensure generated selectors consistently
choose the CUDA-suffixed nvidia-libnvcomp wheel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f862653a-635d-4940-b7b5-87ae814b231d
📒 Files selected for processing (1)
dependencies.yaml
jameslamb
left a comment
There was a problem hiding this comment.
Please, include some context in the PR description. I know it's for rapidsai/pre-commit-hooks#147 (comment) but I don't think anyone else reviewing or finding this from search / git blame in the future would.
I'm personally ok with our dependencies.yaml files getting a bit more verbose like this in the name of being explicit, especially if it helps us to enforce these conventions via a pre-commit hook. I'd like to hear what @bdice says too if he has time to review.
bdice
left a comment
There was a problem hiding this comment.
Thanks @KyleFromNVIDIA and @jameslamb! This looks good. I'm glad we were able to confine the changes to the wheel pipelines and avoid declaring things that touch conda.
I'm personally ok with our
dependencies.yamlfiles getting a bit more verbose like this in the name of being explicit, especially if it helps us to enforce these conventions via a pre-commit hook. I'd like to hear what @bdice says too if he has time to review.
Yes, 100%. This is helpful for enforcing correctness!
|
/merge |
|
/merge |
In #23472, we incorrectly used `RAPIDS_CUDA_VERSION` in `build.sh`. This variable comes from CI only and should not be required to build locally. If `RAPIDS_CUDA_VERSION` is not available, determine the CUDA version in a similar manner to rapids-build-backend: run `nvcc --version` and parse the output. Fixes #23587 Authors: - Kyle Edwards (https://github.com/KyleFromNVIDIA) Approvers: - James Lamb (https://github.com/jameslamb) URL: #23588
Description
In order to support rapidsai/pre-commit-hooks#132, matrix parameters like
use_cuda_wheelsandcuda_suffixedhave to be explicit so that theverify-dependencieshook doesn't produce false positives. Make them explicit so we can use the hook.Checklist