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

remove all remaining uses of cugraph-ops #4784

Merged
merged 10 commits into from
Dec 4, 2024

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Nov 25, 2024

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:

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

How I identified these changes

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:

cugraph/build.sh

Lines 334 to 341 in c5d3d23

for PROJECT in libcugraphops libwholegraph; do
XML_DIR="${REPODIR}/docs/cugraph/${PROJECT}"
rm -rf "${XML_DIR}"
mkdir -p "${XML_DIR}"
export XML_DIR_${PROJECT^^}="$XML_DIR"
echo "downloading xml for ${PROJECT} into ${XML_DIR}. Environment variable XML_DIR_${PROJECT^^} is set to ${XML_DIR}"
curl -O "https://d1664dvumjb44w.cloudfront.net/${PROJECT}/xml_tar/${RAPIDS_VERSION}/xml.tar.gz"

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)

@jameslamb jameslamb added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 25, 2024
@jameslamb
Copy link
Member Author

Hopefully rapidsai/devcontainers#423 will fix the devcontainer CI jobs here.

@@ -49,7 +49,7 @@ jobs:
files_yaml: |
test_cpp:
- '**'
- '!.devcontainers/**'
- '!.devcontainer/**'
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unrelated to cugraph-ops, but just slipping it in here because it's not worth a full CI run on its own.

This repo doesn't have a .devcontainers/ directory... it's called .devcontainer/.

@jameslamb jameslamb changed the title WIP: remove more uses of cugraph-ops WIP: remove all remaining uses of cugraph-ops Nov 26, 2024
@jameslamb
Copy link
Member Author

It looks to me like the docs-build PR CI job for 25.02 is going to be blocked until either wholegraph docs move out of this repo or new wholegraph docs are published from cugraph-gnn.

┌──────────────────────────────────────┐
|    Download libwholegraph xml_tar    |
└──────────────────────────────────────┘

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100   321    0   321    0     0    808      0 --:--:-- --:--:-- --:--:--   808

gzip: stdin: not in gzip format
tar: Child returned status 1
tar: Error is not recoverable: exiting now

(build link)

That's coming from here:

cugraph/ci/build_docs.sh

Lines 56 to 61 in 1f1cace

for PROJECT in libcugraphops libwholegraph; do
rapids-logger "Download ${PROJECT} xml_tar"
TMP_DIR=$(mktemp -d)
export XML_DIR_${PROJECT^^}="$TMP_DIR"
curl "https://d1664dvumjb44w.cloudfront.net/${PROJECT}/xml_tar/${RAPIDS_VERSION_NUMBER}/xml.tar.gz" | tar -xzf - -C "${TMP_DIR}"
done

@alexbarghi-nv @acostadon can you help with this?

@alexbarghi-nv
Copy link
Member

It looks to me like the docs-build PR CI job for 25.02 is going to be blocked until either wholegraph docs move out of this repo or new wholegraph docs are published from cugraph-gnn.

┌──────────────────────────────────────┐
|    Download libwholegraph xml_tar    |
└──────────────────────────────────────┘

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100   321    0   321    0     0    808      0 --:--:-- --:--:-- --:--:--   808

gzip: stdin: not in gzip format
tar: Child returned status 1
tar: Error is not recoverable: exiting now

(build link)

That's coming from here:

cugraph/ci/build_docs.sh

Lines 56 to 61 in 1f1cace

for PROJECT in libcugraphops libwholegraph; do
rapids-logger "Download ${PROJECT} xml_tar"
TMP_DIR=$(mktemp -d)
export XML_DIR_${PROJECT^^}="$TMP_DIR"
curl "https://d1664dvumjb44w.cloudfront.net/${PROJECT}/xml_tar/${RAPIDS_VERSION_NUMBER}/xml.tar.gz" | tar -xzf - -C "${TMP_DIR}"
done

@alexbarghi-nv @acostadon can you help with this?

WholeGraph docs are supposed to move to the docs repo, not cugraph-gnn. Can we temporarily disable the wholegraph docs build until @acostadon finishes the migration?

@jameslamb
Copy link
Member Author

Can we temporarily disable the wholegraph docs build until @acostadon finishes the migration?

Sure, works for me. I just pushed c1f4e56 doing that.

@jameslamb

This comment was marked as resolved.

@jameslamb jameslamb changed the title WIP: remove all remaining uses of cugraph-ops remove all remaining uses of cugraph-ops Dec 2, 2024
@jameslamb jameslamb marked this pull request as ready for review December 2, 2024 23:45
@jameslamb jameslamb requested review from a team as code owners December 2, 2024 23:45
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.

Python looks good.👍

@jameslamb
Copy link
Member Author

Thanks everyone! There's just 1 test job left, so I'm going to queue this for auto-merge.

@jameslamb
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit 5a26158 into rapidsai:branch-25.02 Dec 4, 2024
76 of 79 checks passed
@jameslamb jameslamb deleted the remove-more-cugraph-ops branch December 4, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci CMake conda cuGraph 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