rapidsai / ucx-py

Python bindings for UCX
https://ucx-py.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
121 stars 58 forks source link

split up CUDA-suffixed dependencies in dependencies.yaml #1057

Closed jameslamb closed 3 months ago

jameslamb commented 3 months ago

Description

Contributes to https://github.com/rapidsai/build-planning/issues/31

In short, RAPIDS DLFW builds want to produce wheels with unsuffixed dependencies, e.g. cudf depending on rmm, not rmm-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:

  1. it should be very low-risk
  2. getting these changes into 24.08 prevents the need to carry around patches for every library in DLFW builds using RAPIDS 24.08
jameslamb commented 3 months ago

I've updated this based on the suggestions from https://github.com/rapidsai/cudf/pull/16183.

Ran the following to check update-version.sh.

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 update-version.sh. Pushed those fixes here as well.

jameslamb commented 3 months ago

/merge

jameslamb commented 3 months ago

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.

jakirkham commented 3 months ago

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

jameslamb commented 3 months ago

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 https://github.com/rapidsai/ucx-py/pull/1057#discussion_r1690530680 in 24.08.

That line is just a selector to choose between installing libucx-cu11 and libucx-cu12 in docs-building (an environment that doesn't have CUDA or GPUs), and all that matters there functionally is that it match a dependencies.yaml matrix "12.*".

Functionally, it could be "cuda=12.9999" and still achieve the same goal.

I agree with changing the 12.2 to 12.5 just for the general benefit of defaulting to 12.5 everywhere as a form of subtle documentation about what's supported, but there's no need to rush that change into 24.08.

jakirkham commented 3 months ago

Ok cool. Could it just be 12. then?

Maybe that avoids some confusion and saves future churn

jameslamb commented 3 months ago

@jakirkham @bdice I've put up https://github.com/rapidsai/ucx-py/pull/1059, targeting branch-0.40, with changes that I think address all of your comments here.