Update CuPy and NumPy dependency specs#3058
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughRaises the minimum CuPy version from ChangesCuPy and NumPy Version Bump
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| - cupy-cuda12x[ctk]>=14.0.1,!=14.1.0 | ||
| # fallback to CUDA 13 versions if 'cuda' is '13.*' or not provided | ||
| - matrix: | ||
| packages: | ||
| - cupy-cuda13x>=13.6.0,!=14.0.0,!=14.1.0 | ||
| - cupy-cuda13x[ctk]>=14.0.1,!=14.1.0 | ||
| depends_on_rapids_logger: |
There was a problem hiding this comment.
The depends_on_cupy block is also used to define test_python -- and the requirements generated by dependency-file-generator get piped into $PIP_CONSTRAINTS in test_wheel_pylibraft.sh (and others).
But you can't use extras in the $PIP_CONSTRAINTS -- maybe we break it out into depends_on_cupy and depends_on_cupy_test and then explicitly list the ctk packages there?
There was a problem hiding this comment.
I'm happy with the resolution pushed in recent commits, where the [ctk] extra is only present in output_type: pyproject lists.
| cuda: "12.*" | ||
| packages: | ||
| - cupy-cuda12x>=13.6.0,!=14.0.0,!=14.1.0 | ||
| - cupy-cuda12x[ctk]>=14.0.1,!=14.1.0 |
There was a problem hiding this comment.
Sorry to complicate this, but I think this needs the use_cuda_wheels: true qualifier, like this:
Lines 271 to 283 in c1babf1
Otherwise, this will pull in cuda-toolkit[...]:
https://github.com/cupy/cupy/blob/9d193b65ea0a393b579cb50d5db8dbf23a667ab1/setup.py#L85-L87
And we'll end up having to patch it in builds like DLFW where we prefer a system-installed CTK.
This applies to all of the PRs.
There was a problem hiding this comment.
Thanks for the updates across all the repos to address this, I'm happy with the new state of these PRs: rapidsai/build-planning#279 (comment)
jameslamb
left a comment
There was a problem hiding this comment.
Approved with one tiny suggestion
| - matrix: | ||
| cuda: "12.*" | ||
| packages: | ||
| - cupy-cuda12x>=14.0.1,!=14.1.0 | ||
| # fallback to CUDA 13 versions if 'cuda' is '13.*' or not provided | ||
| - matrix: | ||
| packages: | ||
| - cupy-cuda13x>=14.0.1,!=14.1.0 | ||
| - output_types: pyproject | ||
| matrices: | ||
| - matrix: | ||
| cuda: "12.*" | ||
| use_cuda_wheels: "false" | ||
| packages: | ||
| - cupy-cuda12x>=14.0.1,!=14.1.0 | ||
| - matrix: | ||
| cuda: "13.*" | ||
| use_cuda_wheels: "false" | ||
| packages: | ||
| - cupy-cuda13x>=14.0.1,!=14.1.0 |
There was a problem hiding this comment.
| - matrix: | |
| cuda: "12.*" | |
| packages: | |
| - cupy-cuda12x>=14.0.1,!=14.1.0 | |
| # fallback to CUDA 13 versions if 'cuda' is '13.*' or not provided | |
| - matrix: | |
| packages: | |
| - cupy-cuda13x>=14.0.1,!=14.1.0 | |
| - output_types: pyproject | |
| matrices: | |
| - matrix: | |
| cuda: "12.*" | |
| use_cuda_wheels: "false" | |
| packages: | |
| - cupy-cuda12x>=14.0.1,!=14.1.0 | |
| - matrix: | |
| cuda: "13.*" | |
| use_cuda_wheels: "false" | |
| packages: | |
| - cupy-cuda13x>=14.0.1,!=14.1.0 | |
| - matrix: | |
| cuda: "12.*" | |
| packages: | |
| - &cupy_cu12 cupy-cuda12x>=14.0.1,!=14.1.0 | |
| # fallback to CUDA 13 versions if 'cuda' is '13.*' or not provided | |
| - matrix: | |
| packages: | |
| - &cupy_cu13 cupy-cuda13x>=14.0.1,!=14.1.0 | |
| - output_types: pyproject | |
| matrices: | |
| - matrix: | |
| cuda: "12.*" | |
| use_cuda_wheels: "false" | |
| packages: | |
| - *cupy_cu12 | |
| - matrix: | |
| cuda: "13.*" | |
| use_cuda_wheels: "false" | |
| packages: | |
| - *cupy_cu13 |
Minor comment... realizing that in many of these PRs, the spelling of this dependency is indentical in both of these cases:
requirementspyproject.toml+use_cuda_wheels: "false"
Using YAML anchors could help keep them consistent (I think we always want them to be?) and reduce duplication.
| cuda: "12.*" | ||
| packages: | ||
| - cupy-cuda12x>=13.6.0,!=14.0.0,!=14.1.0 | ||
| - cupy-cuda12x[ctk]>=14.0.1,!=14.1.0 |
There was a problem hiding this comment.
Thanks for the updates across all the repos to address this, I'm happy with the new state of these PRs: rapidsai/build-planning#279 (comment)
Contributes to rapidsai/build-planning#279
RAPIDS is taking on requirements
cupy>=14.0.1,!=14.1.0andnumpy>=2.0. Wheel dependencies oncupy-cuda12xandcupy-cuda13xnow use the[ctk]extra.See the linked issue for details.