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

Jump to bottom

Remove cugraph-equivariance #4762

Merged
merged 6 commits into from
Nov 22, 2024
Merged

Remove cugraph-equivariance #4762

merged 6 commits into from
Nov 22, 2024

Conversation

BradReesWork
Copy link
Member

@BradReesWork BradReesWork commented Nov 14, 2024 •
edited by bdice
Loading

The cugraph-equivariant package was moved into cuEquivariance a few releases ago. This PR removes the code and build processes.

@BradReesWork BradReesWork added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 14, 2024
@BradReesWork BradReesWork added this to the 24.12 milestone Nov 14, 2024
@BradReesWork BradReesWork requested review from a team as code owners November 14, 2024 20:52
jameslamb
jameslamb approved these changes Nov 14, 2024
View reviewed changes
Copy link
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Oh wow! Did not realize this was getting cut this release too!

Have you informed ops that we won't be publishing 24.12 packages of this?

There are some other changes that'll be required too:

alexbarghi-nv
alexbarghi-nv approved these changes Nov 14, 2024
View reviewed changes
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.

👍

This was referenced Nov 15, 2024
Merged
Merged
@jameslamb
Copy link
Member

I put up devcontainers and rapids-metadata changes for you.

That devcontainers PR will need to pass and be merged before CI can succeed here.

And before that can happen, we'll need to get rapidsai/cugraph-gnn#69 merged and new pylibwholegraph packages built. CI takes 2+ hours there and 3+ hours in the devcontainers repo, so best-case it'll be at least 5 hours until CI can pass on this PR.

jameslamb added a commit to rapidsai/rapids-metadata that referenced this pull request Nov 15, 2024
@jameslamb
remove cugraph-equivariant (#33)
01c034a 
I learned yesterday that `cugraph-equivariance` is going to be
completely removed in RAPIDS 24.12:
rapidsai/cugraph#4762

This proposes updating `rapids-metadata` accordingly.
@BradReesWork BradReesWork requested review from a team as code owners November 18, 2024 18:56
@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@BradReesWork BradReesWork requested review from acostadon and removed request for rlratzel November 18, 2024 19:14
@BradReesWork
Copy link
Member Author

merge conflict caused the nx PR to get merged in

@BradReesWork BradReesWork self-assigned this Nov 21, 2024
bdice
bdice reviewed Nov 21, 2024
View reviewed changes
ci/build_wheel.sh Outdated Show resolved Hide resolved
@bdice bdice changed the title Removed equivariance Remove cugraph-equivariance Nov 21, 2024
bdice
bdice approved these changes Nov 21, 2024
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I fixed an issue in the Python build script and removed some leftover files. The upstream CI issue from RAFT kernel symbols should be fixed as well now, so hopefully this round of CI will pass. Thanks, team!

@bdice
Copy link
Contributor

bdice commented Nov 21, 2024

/merge

bdice
bdice reviewed Nov 21, 2024
View reviewed changes
cpp/tests/community/balanced_edge_test.cpp
@@ -15,7 +15,7 @@
#include <rmm/device_vector.hpp>
#include <rmm/exec_policy.hpp>

TEST(balanced_edge, success)
TEST(balanced_edge, DISABLED_success)
Copy link
Contributor

Choose a reason for hiding this comment

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

@ChuckHastings is currently diagnosing the failure here, which is unrelated to the removal of cugraph-equivariance. We think it is related to rapidsai/raft#2481. I'll open a tracking issue for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tracking issue: #4777

@bdice bdice mentioned this pull request Nov 21, 2024
Closed
@rapids-bot rapids-bot bot merged commit 6ad8cf8 into rapidsai:branch-24.12 Nov 22, 2024
71 checks passed
@bdice bdice mentioned this pull request Nov 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice approved these changes

@alexbarghi-nv alexbarghi-nv alexbarghi-nv approved these changes

@wangxiaoyunNV wangxiaoyunNV wangxiaoyunNV approved these changes

@ChuckHastings ChuckHastings ChuckHastings approved these changes

@jameslamb jameslamb jameslamb approved these changes

@tingyu66 tingyu66 tingyu66 approved these changes

@acostadon acostadon Awaiting requested review from acostadon

Assignees

@BradReesWork BradReesWork

Labels
ci conda cuGraph improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Milestone
24.12
Development

Successfully merging this pull request may close these issues.
7 participants
@BradReesWork @jameslamb @bdice @tingyu66 @ChuckHastings @wangxiaoyunNV @alexbarghi-nv