refactor CUDA versions in dependencies.yaml#517
refactor CUDA versions in dependencies.yaml#517rapids-bot[bot] merged 3 commits intorapidsai:branch-24.02from jameslamb:rework-dependencies
Conversation
vyasr
added
improvement
| - output_types: conda | ||
| matrices: | ||
| - matrix: | ||
| cuda: "11.[245]" |
There was a problem hiding this comment.
@jameslamb What do you think of this syntax? This is valid glob syntax and should work since it's handled by fnmatch.fnmatch. Just wanted to point it out explicitly to you since it's different than what we've been writing elsewhere.
There was a problem hiding this comment.
I thought this could lead to surprises if there were ever double-digit releases (11.10+), but I guess that isn't true and [] matches a single character.
On Python 3.10.12:
import fnmatch
fnmatch.fnmatch("11.12", "11.[1245]")
# False
fnmatch.fnmatch("11.12", "11.[1245][12]")
# True
fnmatch.fnmatch("11.12", "11.[245][12]")
# FalseGiven that, I support this and think it's a nice simplification. Thanks for the @!
| - matrix: | ||
| arch: x86_64 | ||
| packages: | ||
| - sysroot_linux-64==2.17 |
There was a problem hiding this comment.
It looks like we were requiring sysroot_linux-64 on ARM builds before this PR. I fixed that.
There was a problem hiding this comment.
makes sense to me! This makes it clearer that the package name is architecture-dependent.
| - matrix: | ||
| cuda: "12.*" | ||
| packages: | ||
| - cuda-cupti-dev |
There was a problem hiding this comment.
We can explore dropping this as a follow-up PR, due to #504.
bdice
left a comment
bdice
left a comment
There was a problem hiding this comment.
Blocking merge since I made a few significant changes (improvements, hopefully).
|
@bdice I support these changes... the tighter globbing, moving non-CUDA things out of the ✅ approved |
|
Changes look good to me. |
|
/merge |
Contributes to rapidsai/build-planning#7. Proposes splitting the `cuda-version` dependency in `dependencies.yaml` out to its own thing, separate from the bits of the CUDA Toolkit this project needs. ### Benefits of this change * prevents accidental inclusion of multiple `cuda-version` version in environments * reduces update effort (via enabling more use of globs like `"12.*"`) * improves the chance that errors like "`conda` recipe is missing a dependency" are caught in CI Authors: - James Lamb (https://github.com/jameslamb) - Bradley Dice (https://github.com/bdice) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Jake Awe (https://github.com/AyodeAwe) - Bradley Dice (https://github.com/bdice) URL: rapidsai#517
* switches to CUDA 12.2.2 for building conda packages and wheels * adds new tests running against CUDA 12.2.2 * adds a `"12.*"` glob in `dependencies.yaml` missed in #517 ### 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: * rapidsai/build-planning#7 * rapidsai/shared-workflows#166 Planning a second round of PRs to revert these references back to a proper `branch-24.{nn}` release branch of `shared-workflows` once rapidsai/shared-workflows#166 is merged. *(created with `rapids-reviser`)* Authors: - James Lamb (https://github.com/jameslamb) - Bradley Dice (https://github.com/bdice) Approvers: - Bradley Dice (https://github.com/bdice) - Jake Awe (https://github.com/AyodeAwe) URL: #521
4 participants
Description
Contributes to rapidsai/build-planning#7.
Proposes splitting the
cuda-versiondependency independencies.yamlout to its own thing, separate from the bits of the CUDA Toolkit this project needs.Benefits of this change
cuda-versionversion in environments"12.*")condarecipe is missing a dependency" are caught in CIChecklist
cmake-format.jsonis up to date with these changes.include_guard(GLOBAL))