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.

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

pylibcugraph: declare cupy and numpy hard dependencies #4854

Merged
merged 8 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
4 changes: 3 additions & 1 deletion conda/recipes/pylibcugraph/meta.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2023-2024, NVIDIA CORPORATION.
# Copyright (c) 2023-2025, NVIDIA CORPORATION.

{% set version = environ['RAPIDS_PACKAGE_VERSION'].lstrip('v') + environ.get('VERSION_SUFFIX', '') %}
{% set minor_version = version.split('.')[0] + '.' + version.split('.')[1] %}
Expand Down Expand Up @@ -74,7 +74,9 @@ requirements:
{% else %}
- cuda-cudart
{% endif %}
- cupy >=12.0.0
- libcugraph ={{ version }}
- numpy>=1.23,<3.0a0
- pylibraft ={{ minor_version }}
- python
- rmm ={{ minor_version }}
Expand Down
26 changes: 15 additions & 11 deletions dependencies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ files:
- depends_on_libcudf
- depends_on_libraft
- depends_on_librmm
- depends_on_numpy
- depends_on_pylibraft
- depends_on_pylibwholegraph
- depends_on_pytorch
Expand All @@ -33,7 +34,6 @@ files:
- test_notebook
- test_python_common
- test_python_cugraph
- test_python_pylibcugraph
checks:
output: none
includes:
Expand Down Expand Up @@ -64,12 +64,12 @@ files:
includes:
- cuda_version
- depends_on_cudf
- depends_on_numpy
- depends_on_pylibwholegraph
- depends_on_pytorch
- py_version
- test_python_common
- test_python_cugraph
- test_python_pylibcugraph
py_build_cugraph:
output: pyproject
pyproject_dir: python/cugraph
Expand Down Expand Up @@ -100,6 +100,7 @@ files:
- depends_on_cupy
- depends_on_dask_cuda
- depends_on_dask_cudf
- depends_on_numpy
- depends_on_pylibcugraph
- depends_on_raft_dask
- depends_on_rmm
Expand All @@ -112,6 +113,7 @@ files:
table: project.optional-dependencies
key: test
includes:
- depends_on_numpy
- depends_on_pylibwholegraph
- test_python_common
- test_python_cugraph
Expand Down Expand Up @@ -141,6 +143,8 @@ files:
table: project
includes:
- cuda_wheels
- depends_on_cupy
- depends_on_numpy
- depends_on_pylibraft
- depends_on_rmm
py_test_pylibcugraph:
Expand All @@ -151,8 +155,8 @@ files:
key: test
includes:
- depends_on_cudf
- depends_on_numpy
- test_python_common
- test_python_pylibcugraph
py_build_cugraph_service_client:
output: pyproject
pyproject_dir: python/cugraph-service/client
Expand All @@ -174,6 +178,7 @@ files:
extras:
table: build-system
includes:
- depends_on_numpy
Copy link
Contributor

Choose a reason for hiding this comment

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

Since numpy doesn’t have any matrices for CUDA version or any extra index URLs, I would treat it as a normal dependency and add it to the corresponding lists. We don’t use depends_on_numpy in other repos because it doesn’t reduce complexity. You could use a YAML anchor for deduplication of the pinning declaration if desired.

Copy link
Contributor

Choose a reason for hiding this comment

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

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 sure.

That is not how I've been thinking about when to do these depends_on_* lists. At https://github.com/rapidsai/build-planning/blob/d9e3c606d95c835ee384ac6480a4af0ac6cb024a/docs/docs/packaging.md#L181 we have this:

Dependencies appearing in several lists should be in their own standalone depends_on_{project} lists.

I can update those docs with this added nuance, I don't disagree with you.

Pushed 6a83f20 here removing depends_on_numpy in favor of anchors. That revealed that there were some other issues in cugrpah-service-server's [test] extra... it was pulling through all of cugraph's testing dependencies, which it doesn't need based on my read of https://github.com/rapidsai/cugraph/tree/branch-25.02/python/cugraph-service/server/cugraph_service_server/testing

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think those docs should be updated. I think it should be something like:

Dependencies appearing in several lists can be handled in two ways. Dependencies with complex requirements such as extra index URLs or package names that vary by CUDA version (like -cuXX suffixes) or format (different conda and PyPI names) should be in their own standalone depends_on_{project} lists. Simpler dependencies can use YAML anchors to avoid duplication of the pinning specs.

I'll re-review and approve based on this. Feel free to update those docs as you see fit.

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 like that language! Put up a PR with it here: rapidsai/build-planning#132

- python_build_rapids
- python_build_wheel
py_run_cugraph_service_server:
Expand All @@ -188,6 +193,7 @@ files:
- depends_on_cupy
- depends_on_dask_cuda
- depends_on_dask_cudf
- depends_on_numpy
- depends_on_rmm
- depends_on_ucx_py
- python_run_cugraph_service_server
Expand Down Expand Up @@ -398,7 +404,6 @@ dependencies:
packages:
- &dask rapids-dask-dependency==25.2.*,>=0.0.0a0
- &numba numba>=0.57
- &numpy numpy>=1.23,<3.0a0
- output_types: conda
packages:
- aiohttp
Expand Down Expand Up @@ -432,7 +437,6 @@ dependencies:
packages:
- *dask
- *numba
- *numpy
- *thrift
test_cpp:
common:
Expand Down Expand Up @@ -462,17 +466,11 @@ dependencies:
packages:
- certifi
- networkx>=2.5.1
- *numpy
- python-louvain
- scikit-learn>=0.23.1
- output_types: [conda]
packages:
- *thrift
test_python_pylibcugraph:
common:
- output_types: [conda, pyproject]
packages:
- *numpy

depends_on_cugraph:
common:
Expand Down Expand Up @@ -765,6 +763,12 @@ dependencies:
- cupy-cuda11x>=12.0.0
- {matrix: null, packages: *cupy_packages_cu11}

depends_on_numpy:
common:
- output_types: [conda, pyproject, requirements]
packages:
- numpy>=1.23,<3.0a0

depends_on_ucx_py:
common:
- output_types: conda
Expand Down
2 changes: 1 addition & 1 deletion python/cugraph-service/server/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
[build-system]

requires = [
"numpy>=1.23,<3.0a0",
Copy link
Member Author

Choose a reason for hiding this comment

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

cugraph-service-server should have a hard runtime dependency on numpy, because of this:

its conda recipe does:

- numpy >=1.23,<3.0a0

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, thanks for being so thorough!

"rapids-build-backend>=0.3.1,<0.4.0.dev0",
"setuptools>=61.0.0",
"wheel",
Expand Down Expand Up @@ -48,7 +49,6 @@ cugraph-service-server = "cugraph_service_server.__main__:main"
test = [
"certifi",
"networkx>=2.5.1",
"numpy>=1.23,<3.0a0",
"pandas",
"pytest",
"pytest-benchmark",
Expand Down
9 changes: 3 additions & 6 deletions python/pylibcugraph/pylibcugraph/bfs.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
# Copyright (c) 2022-2025, NVIDIA CORPORATION.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand All @@ -14,6 +14,8 @@
# Have cython use python 3 syntax
# cython: language_level = 3

import cupy

from libc.stdint cimport uintptr_t
from libc.stdint cimport int32_t
from libc.limits cimport INT_MAX
Expand Down Expand Up @@ -141,11 +143,6 @@ def bfs(ResourceHandle handle, _GPUGraph graph,
>>> })
"""

try:
import cupy
except ModuleNotFoundError:
raise RuntimeError("bfs requires the cupy package, which could not "
"be imported")
assert_CAI_type(sources, "sources")

if depth_limit <= 0:
Expand Down
12 changes: 3 additions & 9 deletions python/pylibcugraph/pylibcugraph/node2vec.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
# Copyright (c) 2022-2025, NVIDIA CORPORATION.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand All @@ -14,6 +14,8 @@
# Have cython use python 3 syntax
# cython: language_level = 3

import cupy

from libc.stdint cimport uintptr_t

from pylibcugraph._cugraph_c.types cimport (
Expand Down Expand Up @@ -124,14 +126,6 @@ def node2vec(ResourceHandle resource_handle,

"""

# FIXME: import these modules here for now until a better pattern can be
# used for optional imports (perhaps 'import_optional()' from cugraph), or
# these are made hard dependencies.
try:
import cupy
except ModuleNotFoundError:
raise RuntimeError("node2vec requires the cupy package, which could not "
"be imported")
assert_CAI_type(seed_array, "seed_array")

cdef cugraph_resource_handle_t* c_resource_handle_ptr = \
Expand Down
19 changes: 4 additions & 15 deletions python/pylibcugraph/pylibcugraph/pagerank.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
# Copyright (c) 2022-2025, NVIDIA CORPORATION.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand All @@ -14,6 +14,9 @@
# Have cython use python 3 syntax
# cython: language_level = 3

import cupy
import numpy

from pylibcugraph._cugraph_c.types cimport (
bool_t,
)
Expand Down Expand Up @@ -167,20 +170,6 @@ def pagerank(ResourceHandle resource_handle,
array([0.11615585, 0.21488841, 0.2988108 , 0.3701449 ], dtype=float32)
"""

# FIXME: import these modules here for now until a better pattern can be
# used for optional imports (perhaps 'import_optional()' from cugraph), or
# these are made hard dependencies.
try:
import cupy
except ModuleNotFoundError:
raise RuntimeError("pagerank requires the cupy package, which could "
"not be imported")
try:
import numpy
except ModuleNotFoundError:
raise RuntimeError("pagerank requires the numpy package, which could "
"not be imported")

cdef cugraph_type_erased_device_array_view_t* \
initial_guess_vertices_view_ptr = \
create_cugraph_type_erased_device_array_view_from_py_obj(
Expand Down
19 changes: 4 additions & 15 deletions python/pylibcugraph/pylibcugraph/personalized_pagerank.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
# Copyright (c) 2022-2025, NVIDIA CORPORATION.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand All @@ -14,6 +14,9 @@
# Have cython use python 3 syntax
# cython: language_level = 3

import cupy
import numpy

from pylibcugraph._cugraph_c.types cimport (
bool_t,
)
Expand Down Expand Up @@ -177,20 +180,6 @@ def personalized_pagerank(ResourceHandle resource_handle,
array([0.00446455, 0.00379487, 0.53607565, 0.45566472 ], dtype=float32)
"""

# FIXME: import these modules here for now until a better pattern can be
# used for optional imports (perhaps 'import_optional()' from cugraph), or
# these are made hard dependencies.
try:
import cupy
except ModuleNotFoundError:
raise RuntimeError("pagerank requires the cupy package, which could "
"not be imported")
try:
import numpy
except ModuleNotFoundError:
raise RuntimeError("pagerank requires the numpy package, which could "
"not be imported")

cdef cugraph_type_erased_device_array_view_t* \
initial_guess_vertices_view_ptr = \
create_cugraph_type_erased_device_array_view_from_py_obj(
Expand Down
19 changes: 4 additions & 15 deletions python/pylibcugraph/pylibcugraph/sssp.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
# Copyright (c) 2022-2025, NVIDIA CORPORATION.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand All @@ -14,6 +14,9 @@
# Have cython use python 3 syntax
# cython: language_level = 3

import cupy
import numpy

from pylibcugraph._cugraph_c.types cimport (
bool_t,
)
Expand Down Expand Up @@ -124,20 +127,6 @@ def sssp(ResourceHandle resource_handle,
array([-1, -1, 1, 2], dtype=int32)
"""

# FIXME: import these modules here for now until a better pattern can be
# used for optional imports (perhaps 'import_optional()' from cugraph), or
# these are made hard dependencies.
try:
import cupy
except ModuleNotFoundError:
raise RuntimeError("sssp requires the cupy package, which could not "
"be imported")
try:
import numpy
except ModuleNotFoundError:
raise RuntimeError("sssp requires the numpy package, which could not "
"be imported")

if compute_predecessors is False:
raise ValueError("compute_predecessors must be True for the current "
"release.")
Expand Down
2 changes: 2 additions & 0 deletions python/pylibcugraph/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ authors = [
license = { text = "Apache 2.0" }
requires-python = ">=3.10"
dependencies = [
"cupy-cuda11x>=12.0.0",
"numpy>=1.23,<3.0a0",
"nvidia-cublas",
"nvidia-curand",
"nvidia-cusolver",
Expand Down
Loading