-
Notifications
You must be signed in to change notification settings - Fork 309
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
Remove GNN Packages #4765
Remove GNN Packages #4765
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pulled this branch and searched like this:
git grep -E -i 'dgl|pyg'
Found some more things that I think should be deleted.
test files:
- https://github.com/rapidsai/cugraph/blob/branch-24.12/ci/run_cugraph_pyg_pytests.sh
- https://github.com/rapidsai/cugraph/blob/branch-24.12/ci/run_cugraph_dgl_pytests.sh
test code:
Lines 102 to 107 in c712a98
echo "Python pytest for cugraph_pyg (single-GPU only)..." | |
conda list | |
cd ${CUGRAPH_ROOT}/python/cugraph-pyg/cugraph_pyg | |
# rmat is not tested because of MG testing | |
pytest -sv -m sg --cache-clear --junitxml=${CUGRAPH_ROOT}/junit-cugraph-pytests.xml -v --cov-config=.coveragerc --cov=cugraph_pyg --cov-report=xml:${WORKSPACE}/python/cugraph_pyg/cugraph-coverage.xml --cov-report term --ignore=raft --benchmark-disable | |
echo "Ran Python pytest for cugraph_pyg : return code was: $?, test script exit code is now: $EXITCODE" |
the conda recipes:
- https://github.com/rapidsai/cugraph/tree/branch-24.12/conda/recipes/cugraph-dgl
- https://github.com/rapidsai/cugraph/tree/branch-24.12/conda/recipes/cugraph-pyg
Test coverage config:
Line 4 in c712a98
cugraph-pyg/cugraph_pyg/* |
Line 8 in c712a98
cugraph-pyg/cugraph_pyg/tests/* |
These should be taken care of now. |
The benchmarks are going to stay for now; those will go away in 25.02. |
CI here is going to fail until some other things are resolved.
|
…v/cugraph into remove-gnn-packages
/merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks right but I admit I'm relying more on the passing CI status and working nightly packages to say for sure.
This repo should no longer need any build-time dependencies on cloning the `cugraph-ops` source code or pulling in its packages, as of these PRs: * #4744 * #4765 This PR proposes the following: * removing CI configuration related to cloning the `cugraph-ops` repo * removing CMake options related to `cugraph-ops` - `ALLOW_CLONE_CUGRAPH_OPS` - `CUGRAPH_EXCLUDE_CUGRAPH_OPS_FROM_ALL` - `CUGRAPH_USE_CUGRAPH_OPS_STATIC` - `USE_CUGRAPH_OPS` * removing dependencies on `pylibcugraphops` / `libcugraphops` in development scripts, conda recipes * removing the `cugraph-ops` docs, and references to them in other docs * removing unused source file `cpp/src/utilities/cugraph_ops_utils.hpp` * removing the `cugraph_ops` pytest marker ## Notes for Reviewers ### Benefits of these changes * faster CI * simplifies the pending work to introduce `libcugraph` wheel packages (rapidsai/rapids-wheels-planning#53) ### How I identified these changes ```shell git grep -i '_ops' git grep -i '\-ops' git grep -i 'cugraphops' ``` ### Why is this so big? I'd originally wanted to leave the docs-building stuff in place, but saw `docs-build` CI here failing because docs haven't been built from the `cugraph-ops` repo yet: https://github.com/rapidsai/cugraph/blob/c5d3d231c9cac361bf51246c13440a42eeda93b9/build.sh#L334-L341 Talked offline with @acostadon and @ChuckHastings , and we agreed that those docs could just be removed here. ### What is intentionally being left? * references in licenses (as some code from `cugraph-ops` is vendored here) * references in the docs for the GNN packages (`cugraph-dgl`, `cugraph-pyg`, `pylibwholegraph`), as those do still rely on `cugraph-ops` (their docs will eventually move out of this repo) Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) - Alex Barghi (https://github.com/alexbarghi-nv) - Chuck Hastings (https://github.com/ChuckHastings) URL: #4784
Removes the GNN packages, which are now part of cugraph-gnn.