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

PR CI: make cugraph builds depend on pylibcugraph builds #4801

Merged
merged 5 commits into from
Dec 9, 2024

Conversation

jameslamb
Copy link
Member

Proposes the following change to the wheel jobs for PR CI:

---
title: Current
---
flowchart LR
    A[build-pylibcugraph] --> B[test-pylibcugraph]
    B --> C[build-cugraph]
    C --> D[test-cugraph]
Loading
---
title: Proposed
---
flowchart LR
    A[build-pylibcugraph] --> B[test-pylibcugraph]
    A --> C[build-cugraph]
    C --> D[test-cugraph]
Loading

Notes for Reviewers

I think reducing the end-to-end time for changes here is even more important now that we have new downstream repos (https://github.com/rapidsai/nx-cugraph and https://github.com/rapidsai/cugraph-gnn) that depend on changes made here.

Benefits of this change

  • shorter end-to-end time for CI runs
    • by parallelizing more work
    • by doing cugraph-cu{11,12} builds more frequently, which should mean more frequent population of the sccache cache
  • faster feedback about cugraph-cu{11,12} build issues
  • consistency with the rest of RAPIDS (every other RAPIDS project has builds depend on builds, not tests, as far as I know)

Costs of this change

  • more at-one-moment load on GPU CI runners as a result of cugraph PRs (wheel tests for cugraph could now occupy 4 GPU runners at once instead of just 2)
    • pylibcugraph test jobs tend to take around 6-10 minutes once scheduled onto a runner, so this shouldn't be too bad

@jameslamb jameslamb added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Dec 4, 2024
@jameslamb jameslamb requested a review from a team as a code owner December 4, 2024 16:05
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Nice catch. This feels like it was always the intended design, comparing to other RAPIDS repos.

@jameslamb jameslamb changed the title pr CI: make cugraph builds depend on pylibcugraph builds PR CI: make cugraph builds depend on pylibcugraph builds Dec 4, 2024
@jameslamb jameslamb requested a review from a team as a code owner December 6, 2024 15:30
@github-actions github-actions bot added the python label Dec 6, 2024
@jameslamb
Copy link
Member Author

All Python test jobs are failing because of this deprecation warning, treated as an error:

E   DeprecationWarning: The cuda.cudart module is deprecated and will be removed in a future release, please switch to use the cuda.bindings.runtime module instead.
full stack trace (click me)
ImportError while loading conftest '/__w/cugraph/cugraph/python/cugraph/cugraph/tests/conftest.py'.
tests/conftest.py:15: in <module>
    from cugraph.testing.mg_utils import (
/opt/conda/envs/test/lib/python3.12/site-packages/cugraph/__init__.py:14: in <module>
    from cugraph.community import (
/opt/conda/envs/test/lib/python3.12/site-packages/cugraph/community/__init__.py:14: in <module>
    from cugraph.community.louvain import louvain
/opt/conda/envs/test/lib/python3.12/site-packages/cugraph/community/louvain.py:15: in <module>
    from cugraph.structure import Graph
/opt/conda/envs/test/lib/python3.12/site-packages/cugraph/structure/__init__.py:14: in <module>
    from cugraph.structure.graph_classes import (
/opt/conda/envs/test/lib/python3.12/site-packages/cugraph/structure/graph_classes.py:15: in <module>
    from .graph_implementation import (
/opt/conda/envs/test/lib/python3.12/site-packages/cugraph/structure/graph_implementation/__init__.py:14: in <module>
    from .simpleGraph import simpleGraphImpl
/opt/conda/envs/test/lib/python3.12/site-packages/cugraph/structure/graph_implementation/simpleGraph.py:14: in <module>
    from cugraph.structure import graph_primtypes_wrapper
graph_primtypes_wrapper.pyx:1: in init cugraph.structure.graph_primtypes_wrapper
    ???
/opt/conda/envs/test/lib/python3.12/site-packages/pylibraft/common/__init__.py:19: in <module>
    from .device_ndarray import device_ndarray
/opt/conda/envs/test/lib/python3.12/site-packages/pylibraft/common/device_ndarray.py:18: in <module>
    import rmm
/opt/conda/envs/test/lib/python3.12/site-packages/rmm/__init__.py:17: in <module>
    from rmm import mr
/opt/conda/envs/test/lib/python3.12/site-packages/rmm/mr.py:14: in <module>
    from rmm.pylibrmm.memory_resource import (
/opt/conda/envs/test/lib/python3.12/site-packages/rmm/pylibrmm/__init__.py:15: in <module>
    from .device_buffer import DeviceBuffer
device_buffer.pyx:1: in init rmm.pylibrmm.device_buffer
    ???
memory_resource.pyx:31: in init rmm.pylibrmm.memory_resource
    ???
cuda/cudart.pyx:13: in init cuda.cudart
    ???
E   DeprecationWarning: The cuda.cudart module is deprecated and will be removed in a future release, please switch to use the cuda.bindings.runtime module instead.

(build link)

Just pushed 1a2590a ignoring that warning, as we've done in some other places.

If we accept that workaround for now, I'll put up an issue here about resolving those warnings so they don't become errors when those deprecations are eventually enforced in future cuda-python versions.

@jameslamb
Copy link
Member Author

/merge

Copy link
Member

@alexbarghi-nv alexbarghi-nv left a comment

Choose a reason for hiding this comment

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

👍

@rapids-bot rapids-bot bot merged commit de05abd into rapidsai:branch-25.02 Dec 9, 2024
72 of 73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants