Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 30, 2024
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
782027a
split up CUDA-suffixed dependencies in dependencies.yaml
jameslamb Jul 24, 2024
d22f7e7
consolidate cuda_suffixed=false blocks
jameslamb Jul 24, 2024
c1a4a80
Merge branch 'branch-24.08' of github.com:rapidsai/cugraph into suffi…
jameslamb Jul 24, 2024
96aac83
update comment
jameslamb Jul 25, 2024
f1b0d6a
update comment
jameslamb Jul 25, 2024
60cfb79
Merge branch 'branch-24.08' into suffix-split
jameslamb Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 86 additions & 43 deletions dependencies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ dependencies:
- requests
- nccl>=2.9.9
- ucx-proc=*=gpu
- &ucx_py ucx-py==0.39.*,>=0.0.0a0
- &ucx_py_unsuffixed ucx-py==0.39.*,>=0.0.0a0
- output_types: pyproject
packages:
# cudf uses fsspec but is protocol independent. cugraph
Expand All @@ -504,15 +504,17 @@ dependencies:
matrices:
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- &ucx_py_cu11 ucx-py-cu11==0.39.*,>=0.0.0a0
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- &ucx_py_cu12 ucx-py-cu12==0.39.*,>=0.0.0a0
- matrix:
packages:
- *ucx_py
- *ucx_py_unsuffixed
python_run_nx_cugraph:
common:
- output_types: [conda, pyproject]
Expand All @@ -530,15 +532,17 @@ dependencies:
matrices:
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- &cugraph_cu11 cugraph-cu11==24.8.*,>=0.0.0a0
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- &cugraph_cu12 cugraph-cu12==24.8.*,>=0.0.0a0
- matrix:
packages:
- &cugraph cugraph==24.8.*,>=0.0.0a0
- &cugraph_unsuffixed cugraph==24.8.*,>=0.0.0a0
python_run_cugraph_pyg:
common:
- output_types: [conda, pyproject]
Expand All @@ -550,15 +554,17 @@ dependencies:
matrices:
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- *cugraph_cu11
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- *cugraph_cu12
- matrix:
packages:
- *cugraph
- *cugraph_unsuffixed
python_run_cugraph_service_client:
common:
- output_types: [conda, pyproject]
Expand All @@ -575,27 +581,29 @@ dependencies:
- *thrift
- output_types: conda
packages:
- *ucx_py
- *ucx_py_unsuffixed
specific:
- output_types: pyproject
matrices:
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- *cugraph_cu11
- cugraph-service-client-cu11==24.8.*,>=0.0.0a0
- *ucx_py_cu11
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- *cugraph_cu12
- cugraph-service-client-cu12==24.8.*,>=0.0.0a0
- *ucx_py_cu12
- matrix:
packages:
- *cugraph
- *cugraph_unsuffixed
- cugraph-service-client==24.8.*,>=0.0.0a0
- *ucx_py
- *ucx_py_unsuffixed
test_cpp:
common:
- output_types: conda
Expand Down Expand Up @@ -630,7 +638,7 @@ dependencies:
- scikit-learn>=0.23.1
- output_types: [conda]
packages:
- &pylibwholegraph_conda pylibwholegraph==24.8.*,>=0.0.0a0
- &pylibwholegraph_unsuffixed pylibwholegraph==24.8.*,>=0.0.0a0
- *thrift
test_python_pylibcugraph:
common:
Expand All @@ -648,15 +656,15 @@ dependencies:
common:
- output_types: [conda]
packages:
- cugraph==24.8.*,>=0.0.0a0
- *cugraph_unsuffixed
- pytorch>=2.0
- pytorch-cuda==11.8
- dgl>=1.1.0.cu*
cugraph_pyg_dev:
common:
- output_types: [conda]
packages:
- cugraph==24.8.*,>=0.0.0a0
- *cugraph_unsuffixed
- pytorch>=2.0
- pytorch-cuda==11.8
- &tensordict tensordict>=0.1.2
Expand All @@ -666,7 +674,7 @@ dependencies:
common:
- output_types: [conda]
packages:
- &pytorch_conda pytorch>=2.0,<2.2.0a0
- &pytorch_unsuffixed pytorch>=2.0,<2.2.0a0

specific:
- output_types: [requirements]
Expand Down Expand Up @@ -694,7 +702,7 @@ dependencies:
common:
- output_types: conda
packages:
- *pylibwholegraph_conda
- *pylibwholegraph_unsuffixed
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -703,19 +711,23 @@ dependencies:
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- pylibwholegraph-cu12==24.8.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- pylibwholegraph-cu11==24.8.*,>=0.0.0a0
- {matrix: null, packages: [*pylibwholegraph_conda]}
- {matrix: null, packages: [*pylibwholegraph_unsuffixed]}

depends_on_rmm:
common:
- output_types: conda
packages:
- &rmm_conda rmm==24.8.*,>=0.0.0a0
- &rmm_unsuffixed rmm==24.8.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -724,19 +736,23 @@ dependencies:
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- rmm-cu12==24.8.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- rmm-cu11==24.8.*,>=0.0.0a0
- {matrix: null, packages: [*rmm_conda]}
- {matrix: null, packages: [*rmm_unsuffixed]}

depends_on_cudf:
common:
- output_types: conda
packages:
- &cudf_conda cudf==24.8.*,>=0.0.0a0
- &cudf_unsuffixed cudf==24.8.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -745,19 +761,23 @@ dependencies:
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- cudf-cu12==24.8.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- cudf-cu11==24.8.*,>=0.0.0a0
- {matrix: null, packages: [*cudf_conda]}
- {matrix: null, packages: [*cudf_unsuffixed]}

depends_on_dask_cudf:
common:
- output_types: conda
packages:
- &dask_cudf_conda dask-cudf==24.8.*,>=0.0.0a0
- &dask_cudf_unsuffixed dask-cudf==24.8.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -766,19 +786,23 @@ dependencies:
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- dask-cudf-cu12==24.8.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- dask-cudf-cu11==24.8.*,>=0.0.0a0
- {matrix: null, packages: [*dask_cudf_conda]}
- {matrix: null, packages: [*dask_cudf_unsuffixed]}

depends_on_pylibraft:
common:
- output_types: conda
packages:
- &pylibraft_conda pylibraft==24.8.*,>=0.0.0a0
- &pylibraft_unsuffixed pylibraft==24.8.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -787,19 +811,23 @@ dependencies:
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- pylibraft-cu12==24.8.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- pylibraft-cu11==24.8.*,>=0.0.0a0
- {matrix: null, packages: [*pylibraft_conda]}
- {matrix: null, packages: [*pylibraft_unsuffixed]}

depends_on_raft_dask:
common:
- output_types: conda
packages:
- &raft_dask_conda raft-dask==24.8.*,>=0.0.0a0
- &raft_dask_unsuffixed raft-dask==24.8.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -808,19 +836,23 @@ dependencies:
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- raft-dask-cu12==24.8.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- raft-dask-cu11==24.8.*,>=0.0.0a0
- {matrix: null, packages: [*raft_dask_conda]}
- {matrix: null, packages: [*raft_dask_unsuffixed]}

depends_on_pylibcugraph:
common:
- output_types: conda
packages:
- &pylibcugraph_conda pylibcugraph==24.8.*,>=0.0.0a0
- &pylibcugraph_unsuffixed pylibcugraph==24.8.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -829,19 +861,23 @@ dependencies:
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- pylibcugraph-cu12==24.8.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- pylibcugraph-cu11==24.8.*,>=0.0.0a0
- {matrix: null, packages: [*pylibcugraph_conda]}
- {matrix: null, packages: [*pylibcugraph_unsuffixed]}

depends_on_pylibcugraphops:
common:
- output_types: conda
packages:
- &pylibcugraphops_conda pylibcugraphops==24.8.*,>=0.0.0a0
- &pylibcugraphops_unsuffixed pylibcugraphops==24.8.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -850,19 +886,26 @@ dependencies:
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- pylibcugraphops-cu12==24.8.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- pylibcugraphops-cu11==24.8.*,>=0.0.0a0
- {matrix: null, packages: [*pylibcugraphops_conda]}
- {matrix: null, packages: [*pylibcugraphops_unsuffixed]}

depends_on_cupy:
common:
- output_types: conda
packages:
- cupy>=12.0.0
# NOTE: this is not broken into groups by a 'cuda_suffixed' selector like
# other packages with -cu{nn}x suffixes in this file because devcontainers-based
# builds that build it from source expect a package with a `-cuda{nn}x` suffix
Copy link
Contributor

@bdice bdice Jul 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that build it from source

The antecedent is a little unclear here. "it" refers to cugraph, not cupy here. We never build cupy from source in RAPIDS. Maybe we can simplify the comment like:

Suggested change
# NOTE: this is not broken into groups by a 'cuda_suffixed' selector like
# other packages with -cu{nn}x suffixes in this file because devcontainers-based
# builds that build it from source expect a package with a `-cuda{nn}x` suffix
# cupy is not built from source in devcontainers, so cuda_suffixed isn't needed

or even

Suggested change
# NOTE: this is not broken into groups by a 'cuda_suffixed' selector like
# other packages with -cu{nn}x suffixes in this file because devcontainers-based
# builds that build it from source expect a package with a `-cuda{nn}x` suffix
# cupy is not built from source so cuda_suffixed is not needed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My choice to say "devcontainers-based builds" was a bad one. It's very unclear.

I should have said "DLFW builds".

We do not build cupy from source in RAPIDS devcontainers, but we DO build it from source in the RAPIDS DLFW builds. And there, pass the following through to pip wheel.

--build-option="--cupy-package-name=cupy-cuda$(cut -d'.' -f1 <<< "${CUDA_VERSION}")x"

Here's where cupy exposes that option in its configuration:

https://github.com/cupy/cupy/blob/9381230cc55a5bf96bf838a6c6cc077b38fe68d1/install/cupy_builder/_context.py#L81-L83

So it's important, in the DFLW context, that cugraph depend on e.g. "cupy-cuda12x", not "cupy".

Maybe we should change that in DLFW in the future, or maybe the cupy dependency should be broken out into a separate block called like depends_on_cupy here in dependencies.yaml, so that these cuda_suffixed: "false" entries could be removed.

But with code freeze for 24.08 so close, I wasn't looking to change any behavior... just to reflect the current state in the smallest, least controversial changeset possible to get these merged in.

Given all that... would you support leaving this as-is and instead having a comment like the one I'm proposing on cuml's cupy dependency over in rapidsai/cuml#5974?

# NOTE: cupy still has a "-cuda12x" suffix here, because it's suffixed
#       in DLFW builds

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are on the same page with everything but the wording of this comment -- the behavior here is what I expect, but I don't think the comment needs to describe only DLFW.

To the best of my knowledge, there is no such pip package called cupy (public or internal/DLFW), so there's never a case where we want cupy to be unsuffixed in requirements or pyproject lists. cupy is only ever used as the conda package name.

Note that for RAPIDS, devcontainers (including outside of DLFW) need the unsuffixed name because that matches the exclusion list.

So either way, nothing here is DLFW-specific, if I understand correctly. We should be able to reword this to not mention DLFW and just say devcontainers. Perhaps the "from source" piece of my previous suggestion is irrelevant.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think about it and merge whatever you think fits best for the comment. The behavior is what I expect, so I don't want to get stuck on the comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok thanks!

I was thinking cugraph went into code freeze today, but I realize that wasn't correct. So we do have a bit more time with this one to make it look the way we long-term want it to. I'll push a change here (tomorrow) with a separated-out depends_on_cupy list and a less-confusing comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need the unsuffixed name because that matches the exclusion list.

And just to note... the exclusion list there includes both the suffixed and unsuffixed versions. That's what this line there does:

pip_noinstall+=("${pkg}" "${pkg}-cu.*");

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, cupy is not captured by that exclusion list -- that list is constructed from RAPIDS packages (things in manifest.yaml). cupy is handled in a special case here, where we transform it into:
cupy-cuda[0-9]+x -> cupy-cuda${cuda_version_major}x

This is to work around issues in previous releases' dependencies.yaml that always output cupy-cuda11x instead of outputting the correct version based on the cuda matrix entry.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying that, appreciated!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed, updating the comment to this:

# NOTE: This is intentionally not broken into groups by a 'cuda_suffixed' selector like
#       other packages with -cu{nn}x suffixes in this file.
#       All RAPIDS wheel builds (including in devcontainers) expect cupy to be suffixed.

Hopefully that more clearly captures the intent.

...maybe the cupydependency should be broken out into a separate block called like depends_on_cupy here in dependencies.yaml

🤦🏻 that's already how it works here in cugraph, sorry... too many very-similar-looking PRs yesterday.

specific:
- output_types: [requirements, pyproject]
matrices:
Expand Down
1 change: 1 addition & 0 deletions python/cugraph-dgl/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,4 @@ include = [
[tool.rapids-build-backend]
build-backend = "setuptools.build_meta"
dependencies-file = "../../dependencies.yaml"
matrix-entry = "cuda_suffixed=true"
1 change: 1 addition & 0 deletions python/cugraph-equivariant/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,4 @@ include = [
[tool.rapids-build-backend]
build-backend = "setuptools.build_meta"
dependencies-file = "../../dependencies.yaml"
matrix-entry = "cuda_suffixed=true"
1 change: 1 addition & 0 deletions python/cugraph-pyg/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,4 @@ include = [
[tool.rapids-build-backend]
build-backend = "setuptools.build_meta"
dependencies-file = "../../dependencies.yaml"
matrix-entry = "cuda_suffixed=true"
1 change: 1 addition & 0 deletions python/cugraph-service/client/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ include = [
build-backend = "setuptools.build_meta"
dependencies-file = "../../../dependencies.yaml"
disable-cuda = true
matrix-entry = "cuda_suffixed=true"
1 change: 1 addition & 0 deletions python/cugraph-service/server/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,4 @@ include = [
[tool.rapids-build-backend]
build-backend = "setuptools.build_meta"
dependencies-file = "../../../dependencies.yaml"
matrix-entry = "cuda_suffixed=true"
1 change: 1 addition & 0 deletions python/cugraph/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,4 @@ requires = [
"rmm==24.8.*,>=0.0.0a0",
] # This list was generated by `rapids-dependency-file-generator`. To make changes, edit ../../dependencies.yaml and run `rapids-dependency-file-generator`.
dependencies-file = "../../dependencies.yaml"
matrix-entry = "cuda_suffixed=true"
Loading
Loading