Skip to content
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

remove 'wget' conda dependency, re-organize dependencies.yaml #4805

Merged
merged 12 commits into from
Dec 20, 2024
Merged

remove 'wget' conda dependency, re-organize dependencies.yaml #4805

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
303fe40
depend on librmm for wheel builds, clean up dependencies.yaml
jameslamb Dec 5, 2024
5b0f6f8
Merge branch 'branch-25.02' into dependencies-cleanup
jameslamb Dec 9, 2024
795d4b3
Merge branch 'branch-25.02' into dependencies-cleanup
jameslamb Dec 9, 2024
1c60a03
Merge branch 'branch-25.02' into dependencies-cleanup
nv-rliu Dec 12, 2024
7d77078
fix conflicts
jameslamb Dec 16, 2024
8e72623
Merge branch 'branch-25.02' into dependencies-cleanup
jameslamb Dec 18, 2024
14bcff3
revert out librmm changes
jameslamb Dec 18, 2024
64d0fad
break more things out into depends_on blocks
jameslamb Dec 20, 2024
3b9bd14
dask-cuda is unsuffixed
jameslamb Dec 20, 2024
3aaf837
put back == to cut a few lines out of the diff
jameslamb Dec 20, 2024
7fc9473
remove cugraph-service-client suffixes
jameslamb Dec 20, 2024
34697a6
align thriftpy2 floors, remove libraft-headers
jameslamb Dec 20, 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
1 change: 0 additions & 1 deletion conda/environments/all_cuda-118_arch-x86_64.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,5 @@ dependencies:
- torchmetrics
- ucx-proc=*=gpu
- ucx-py==0.42.*,>=0.0.0a0
- wget
- wheel
name: all_cuda-118_arch-x86_64
1 change: 0 additions & 1 deletion conda/environments/all_cuda-125_arch-x86_64.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,5 @@ dependencies:
- torchmetrics
- ucx-proc=*=gpu
- ucx-py==0.42.*,>=0.0.0a0
- wget
- wheel
name: all_cuda-125_arch-x86_64
80 changes: 51 additions & 29 deletions dependencies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,19 @@ files:
- cpp_build
- cuda
- cuda_version
- docs
- python_build_rapids
- python_build_wheel
- python_build_cythonize
- depends_on_rmm
- depends_on_cudf
- depends_on_cupy
- depends_on_dask_cudf
- depends_on_librmm
- depends_on_pylibraft
- depends_on_raft_dask
- depends_on_pylibwholegraph
- depends_on_cupy
- depends_on_pytorch
- depends_on_raft_dask
- depends_on_rmm
- docs
- python_build_cythonize
- python_build_rapids
- python_build_wheel
- python_run_cugraph
- test_notebook
- test_python_common
Expand All @@ -48,6 +49,7 @@ files:
output: none
includes:
- cuda_version
- depends_on_pylibwholegraph
- py_version
- test_notebook
- test_python_common
Expand All @@ -69,8 +71,8 @@ files:
extras:
table: build-system
includes:
- python_build_rapids
- python_build_cythonize
- python_build_rapids
py_rapids_build_cugraph:
output: pyproject
pyproject_dir: python/cugraph
Expand All @@ -79,21 +81,21 @@ files:
key: requires
includes:
- common_build
- depends_on_rmm
- depends_on_pylibraft
- depends_on_pylibcugraph
- depends_on_pylibraft
- depends_on_rmm
py_run_cugraph:
output: pyproject
pyproject_dir: python/cugraph
extras:
table: project
includes:
- depends_on_rmm
- depends_on_cudf
- depends_on_cupy
- depends_on_dask_cudf
- depends_on_raft_dask
- depends_on_pylibcugraph
- depends_on_cupy
- depends_on_raft_dask
- depends_on_rmm
- python_run_cugraph
py_test_cugraph:
output: pyproject
Expand All @@ -102,17 +104,17 @@ files:
table: project.optional-dependencies
key: test
includes:
- depends_on_pylibwholegraph
- test_python_common
- test_python_cugraph
- depends_on_pylibwholegraph
py_build_pylibcugraph:
output: pyproject
pyproject_dir: python/pylibcugraph
extras:
table: build-system
includes:
- python_build_rapids
- python_build_cythonize
- python_build_rapids
py_rapids_build_pylibcugraph:
output: pyproject
pyproject_dir: python/pylibcugraph
Expand All @@ -121,17 +123,17 @@ files:
key: requires
includes:
- common_build
- depends_on_rmm
- depends_on_pylibraft
- depends_on_rmm
py_run_pylibcugraph:
output: pyproject
pyproject_dir: python/pylibcugraph
extras:
table: project
includes:
- cuda_wheels
- depends_on_rmm
- depends_on_pylibraft
- depends_on_rmm
py_test_pylibcugraph:
output: pyproject
pyproject_dir: python/pylibcugraph
Expand Down Expand Up @@ -171,10 +173,10 @@ files:
extras:
table: project
includes:
- depends_on_rmm
- depends_on_cudf
- depends_on_dask_cudf
- depends_on_cupy
- depends_on_dask_cudf
- depends_on_rmm
- python_run_cugraph_service_server
py_test_cugraph_service_server:
output: pyproject
Expand Down Expand Up @@ -293,7 +295,6 @@ dependencies:
- libcudf==25.2.*,>=0.0.0a0
- libraft-headers==25.2.*,>=0.0.0a0
- libraft==25.2.*,>=0.0.0a0
- librmm==25.2.*,>=0.0.0a0
- openmpi # Required for building cpp-mgtests (multi-GPU tests)
specific:
- output_types: [conda]
Expand Down Expand Up @@ -449,19 +450,19 @@ dependencies:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- &cugraph_cu11 cugraph-cu11==25.2.*,>=0.0.0a0
- cugraph-cu11==25.2.*,>=0.0.0a0
- cugraph-service-client-cu11==25.2.*,>=0.0.0a0
- *ucx_py_cu11
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- &cugraph_cu12 cugraph-cu12==25.2.*,>=0.0.0a0
- cugraph-cu12==25.2.*,>=0.0.0a0
- cugraph-service-client-cu12==25.2.*,>=0.0.0a0
- *ucx_py_cu12
- matrix:
packages:
- &cugraph_unsuffixed cugraph==25.2.*,>=0.0.0a0
- cugraph==25.2.*,>=0.0.0a0
- cugraph-service-client==25.2.*,>=0.0.0a0
- *ucx_py_unsuffixed
test_cpp:
Expand All @@ -476,9 +477,6 @@ dependencies:
- certifi
- ipython
- notebook>=0.5.0
- output_types: [conda]
packages:
- wget
test_python_common:
common:
- output_types: [conda, pyproject]
Expand All @@ -500,7 +498,6 @@ dependencies:
- scikit-learn>=0.23.1
- output_types: [conda]
packages:
- &pylibwholegraph_unsuffixed pylibwholegraph==25.2.*,>=0.0.0a0
- *thrift
test_python_pylibcugraph:
common:
Expand All @@ -512,7 +509,7 @@ dependencies:
common:
- output_types: [conda]
packages:
- &pytorch_conda pytorch>=2.3,<2.5a0
- pytorch>=2.3,<2.5a0
- torchdata
- pydantic
- ogb
Expand Down Expand Up @@ -544,7 +541,7 @@ dependencies:
common:
- output_types: conda
packages:
- *pylibwholegraph_unsuffixed
- &pylibwholegraph_unsuffixed pylibwholegraph==25.2.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
Expand All @@ -565,6 +562,31 @@ dependencies:
- pylibwholegraph-cu11==25.2.*,>=0.0.0a0
- {matrix: null, packages: [*pylibwholegraph_unsuffixed]}

depends_on_librmm:
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want the same for other packages too? libcudf, libraft? It’s okay to defer on this in the name of gradual improvement.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, we can go further in this PR. I just pushed 64d0fad moving more of the RAPIDS dependencies into these depends_on_* lists.

Some notes on other changes in that commit:

  • proposes removing explicit libraft-headers listing
    • libraft is explicitly listed, and directly pulls that in (recipe link)
    • normally I'd advise against relying on transitive relationships like this implicitly, but in this case I think it's ok... the libraft conda package's purpose is to pull in the compiled bits of RAFT and the headers needed to link against them
    • if you disagree I'll happily add a depends_on_libraft_headers, don't feel THAT strongly about it either way
  • removes pyproject and requirements entries for libcudf, libraft, and librmm

Requesting a re-review, that's too much to merge on a stale approval.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is okay to use libraft to get its headers transitively.

The other bits are fine, too. I’ll re-review, just make sure update-version.sh and conda recipes reflect things correctly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright great, thanks.

update version.sh and conda recipes

Just did that in 34697a6.

  • removed libraft-headers from conda recipes and update-version.sh
  • put a floor on thriftpy2 in dependencies.yaml to match what conda recipes were using

common:
- output_types: conda
packages:
- &librmm_unsuffixed librmm==25.2.*,>=0.0.0a0
- output_types: requirements
packages:
# pip recognizes the index as a global option for the requirements.txt file
- --extra-index-url=https://pypi.nvidia.com
- --extra-index-url=https://pypi.anaconda.org/rapidsai-wheels-nightly/simple
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- librmm-cu12==25.2.*,>=0.0.0a0
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- librmm-cu11==25.2.*,>=0.0.0a0
- {matrix: null, packages: [*librmm_unsuffixed]}

depends_on_rmm:
common:
- output_types: conda
Expand Down
Loading