split up CUDA-suffixed dependencies in dependencies.yaml #1057
Conversation
jameslamb
changed the title
|
I've updated this based on the suggestions from rapidsai/cudf#16183. Ran the following to check git fetch upstream --tags
ci/release/update-version.sh '24.10.00'
git grep -E '24\.8|24\.08|0\.39'That revealed a few other places that needed updates in |
| - *cudf_unsuffixed | ||
| # NOTE: cupy still has a "-cuda11x" suffix here, because it's suffixed | ||
| # in DLFW builds | ||
| - *cupy_cu11 |
There was a problem hiding this comment.
This matrix looks like it hasn't been updated to use fallback the way that the other repo's PR have, but is that intentional here because of the need to specify the cupy dep?
There was a problem hiding this comment.
yes exactly. More context: rapidsai/cugraph#4552 (comment)
There was a problem hiding this comment.
If you want, it might be nice to refactor this to a "depends on cudf" / "depends on cupy" structure like we have for other repos.
There was a problem hiding this comment.
I agree, I mentioned that in that comment I linked above (rapidsai/cugraph#4552 (comment)).
In the interest of time and not wanting to go through another CI cycle, I'll defer that to a follow-up PR.
| @@ -10,7 +10,7 @@ build: | |||
| # and therefore tested in this no-CUDA environment | |||
| - | | |||
| pip install \ | |||
| -C rapidsai.matrix-entry="cuda=12.2" \ | |||
| -C rapidsai.matrix-entry="cuda=12.2;cuda_suffixed=true" \ | |||
There was a problem hiding this comment.
Was this missed in an upgrade to 12.5? Feel free to slip that in here or in a follow-up PR.
| -C rapidsai.matrix-entry="cuda=12.2;cuda_suffixed=true" \ | |
| -C rapidsai.matrix-entry="cuda=12.5;cuda_suffixed=true" \ |
There was a problem hiding this comment.
Probably, yes. I'll put up a follow-up PR fixing this... against 24.10. There's not a functional difference for ucx-py, since this is just a way to matrix matrices with wildcard selectors like "12.*".
| - *cudf_unsuffixed | ||
| # NOTE: cupy still has a "-cuda11x" suffix here, because it's suffixed | ||
| # in DLFW builds | ||
| - *cupy_cu11 |
There was a problem hiding this comment.
If you want, it might be nice to refactor this to a "depends on cudf" / "depends on cupy" structure like we have for other repos.
|
/merge |
|
I merged this in the interest of reducing the amount of in-flight stuff ahead of code freeze. @bdice I made myself a reminder to put up a follow-up PR against 24.10 implementing your suggestions. Don't want you to think I was ignoring them... I am just being really conservative with pushing new commits given how oversubscribed our CI runners are right now. |
|
Just know that RAPIDS 24.08 already includes 12.5 So if we need this to work with RAPIDS 24.08 CUDA 12.5, we would need that fix in 24.08 |
It's not necessary to address #1057 (comment) in 24.08. That line is just a selector to choose between installing Functionally, it could be I agree with changing the |
|
Ok cool. Could it just be Maybe that avoids some confusion and saves future churn |
|
@jakirkham @bdice I've put up #1059, targeting |
…12 in fallback dependency lists (#1059) Follow-up to #1057. Implements the following suggestions based on review comments from @jakirkham and @bdice there: * splits `cupy` out into its own list in `dependencies.yaml`, to allow removing some `cuda_suffixed: "false"` blocks that were otherwise redundant * passes `--matrix-entry 'cuda=12.x'` in docs builds, to make it clearer that those builds are just trying to match `cuda: "12.*"` and are not at all dependent on any particular minor version of CUDA Also proposes defaulting to `cupy-cuda12x` in `pyproject.toml`, as part of rapidsai/build-planning#68. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #1059
Description
Contributes to rapidsai/build-planning#31
In short, RAPIDS DLFW builds want to produce wheels with unsuffixed dependencies, e.g.
cudfdepending onrmm, notrmm-cu12.This PR is part of a series across all of RAPIDS to try to support that type of build by setting up CUDA-suffixed and CUDA-unsuffixed dependency lists in
dependencies.yaml.For more details, see:
Notes for Reviewers
Why target 24.08?
This is targeting 24.08 because: