Updates to build and test `nx-cugraph` wheel as part of CI and nightly workflows #3852

rlratzel · 2023-09-08T02:18:15Z

closes rapidsai/graph_dl#302

Updates GHA yaml files to build and test a nx-cugraph wheel
Adds CI scripts for building and testing the nx-cugraph wheel
Adds a smoketest script for nx-cugraph
Relevant code cleanup: removes unnecessary dataset download from cugraph wheel testing

…unnecessary dataset download from cugraph wheel testing.

…ing mamba change

…nd breaking mamba change" This reverts commit 9ba4595.

…uild to work around mamba breaking change.

…0-nx_cugraph_wheel

rlratzel · 2023-09-08T16:10:53Z

/ok to test

…0-nx_cugraph_wheel

ci/wheel_smoke_test_nx-cugraph.py

…0-nx_cugraph_wheel

…he script, uses ls instead of echo to expand wildcards so errors for missing files are more obvious.

…0-nx_cugraph_wheel

rlratzel · 2023-09-22T20:42:01Z

ci/test_wheel.sh

@@ -9,9 +9,9 @@ package_dir=$2
 mkdir -p ./dist
 RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"

-# echo to expand wildcard before adding `[extra]` requires for pip


There was a case where the wildcard did not match the .whl files on disk and 'echo' simply echo'd the wildcard pattern, which made pip generate a misleading error message. Using 'ls' will result in the script erroring out with a clear message about the missing/mis-named files.

Odd, but I'm fine with the change if it works.

I agree with this change, I see the problem with shell wildcard expansion.

…0-nx_cugraph_wheel

vyasr

Couple of notes, but looks fine otherwise.

ci/check_style.sh

vyasr · 2023-09-26T20:56:53Z

ci/test_wheel.sh

@@ -9,9 +9,9 @@ package_dir=$2
 mkdir -p ./dist
 RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"

-# echo to expand wildcard before adding `[extra]` requires for pip


Odd, but I'm fine with the change if it works.

vyasr · 2023-09-26T20:58:08Z

ci/test_wheel_cugraph.sh

-    pushd ./datasets
-    bash ./get_test_data.sh
-    popd
-fi


I guess this change is necessary for nightlies? Were nightly tests for arm not running the tests that required datasets?

We shouldn't remove this, but might need to modify it to only be skipped in ARM PR jobs. In ARM PR jobs, we can only run "smoke tests" due to limited resource capacity.

cugraph/ci/test_wheel.sh

Line 16 in 5c34d3d

# Run smoke tests for aarch64 pull requests

Downloading the datasets is time-intensive and should not be performed for "smoke tests" in ARM PR jobs because it wastes a lot of time on the ARM GPU node.

I'm terribly sorry. I read this backwards. This removed downloading datasets for ALL jobs (all arches, nightly/PR). Are we not dependent on datasets for tests to pass anymore?

The get_test_data.sh script will download supplemental datasets that are too big to commit to our github repo. Those are currently only used by C++ tests. The python tests use the smaller .csv datasets committed to the repo here, so it's safe to skip downloading when only testing python code.

I only removed it here since it was in the proximity of the changes for the wheel builds, but I think an audit of what runs require downloading the supplemental datasets would be good for a separate PR.

Prior to the recent commits to the PR which triggered CI running now, we had everything passing with the download step removed.

Thanks @rlratzel, I'll approve now.

bdice

Requesting changes. We must skip dataset downloads for ARM smoke tests (ARM PRs).

bdice · 2023-09-26T21:23:51Z

ci/test_wheel.sh

@@ -9,9 +9,9 @@ package_dir=$2
 mkdir -p ./dist
 RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"

-# echo to expand wildcard before adding `[extra]` requires for pip


I agree with this change, I see the problem with shell wildcard expansion.

bdice · 2023-09-26T21:27:22Z

ci/test_wheel_cugraph.sh

-    pushd ./datasets
-    bash ./get_test_data.sh
-    popd
-fi


We shouldn't remove this, but might need to modify it to only be skipped in ARM PR jobs. In ARM PR jobs, we can only run "smoke tests" due to limited resource capacity.

cugraph/ci/test_wheel.sh

Line 16 in 5c34d3d

# Run smoke tests for aarch64 pull requests

Downloading the datasets is time-intensive and should not be performed for "smoke tests" in ARM PR jobs because it wastes a lot of time on the ARM GPU node.

rlratzel · 2023-09-27T01:59:52Z

/merge

A couple PRs were merged after `branch-23.12` was created and contained RAPIDS versions that need to be updated in `branch-23.12`. Ref: - #3838 - #3852 Authors: - Ray Douglass (https://github.com/raydouglass) - Rick Ratzel (https://github.com/rlratzel) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Brad Rees (https://github.com/BradReesWork) URL: #3905

Updates GHA yaml files to build and test a nx-cugraph wheel, removes …

985e921

…unnecessary dataset download from cugraph wheel testing.

rlratzel added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 8, 2023

rlratzel added this to the 23.10 milestone Sep 8, 2023

rlratzel self-assigned this Sep 8, 2023

rlratzel added 3 commits September 8, 2023 10:53

Changed rapids-mamba-retry to rapids-conda-retry to work around break…

9ba4595

…ing mamba change

Revert "Changed rapids-mamba-retry to rapids-conda-retry to work arou…

d63eb6e

…nd breaking mamba change" This reverts commit 9ba4595.

Replaces rapids-mamba-retry mambabuild with rapids-conda-retry mambab…

89e9dc9

…uild to work around mamba breaking change.

This comment was marked as resolved.

Sign in to view

Merge remote-tracking branch 'upstream/branch-23.10' into branch-23.1…

f3ffacc

…0-nx_cugraph_wheel

rlratzel added 4 commits September 8, 2023 13:40

chmod +x to new nx-cugraph wheel scripts.

64a3d8c

Merge remote-tracking branch 'upstream/branch-23.10' into branch-23.1…

b3c595f

…0-nx_cugraph_wheel

Adds wheel smoke test scrip for nx-cugraph.

e473c5b

Adds comments to smoke test script.

e67a3ee

This comment was marked as duplicate.

Sign in to view

Cleanup.

de14d8d

This comment was marked as duplicate.

Sign in to view

eriknw reviewed Sep 8, 2023

View reviewed changes

ci/wheel_smoke_test_nx-cugraph.py Outdated Show resolved Hide resolved

rlratzel added 3 commits September 21, 2023 21:44

Merge remote-tracking branch 'upstream/branch-23.10' into branch-23.1…

9412d20

…0-nx_cugraph_wheel

Using nxcg import convention.

52ffb67

Skips running auditwheel on nx-cugraph wheel.

f79f231

This comment was marked as duplicate.

Sign in to view

rlratzel added 2 commits September 22, 2023 10:34

Modifies wheel upload based on if auditwheel needs to be run or not.

0fbddeb

Merge remote-tracking branch 'upstream/branch-23.10' into branch-23.1…

a1d079d

…0-nx_cugraph_wheel

This comment was marked as duplicate.

Sign in to view

Changes conditional logic for clarity.

b3faa5d

This comment was marked as duplicate.

Sign in to view

rlratzel added 2 commits September 22, 2023 14:04

Changes package name passed to test script to match expectations of t…

0e9c754

…he script, uses ls instead of echo to expand wildcards so errors for missing files are more obvious.

Merge remote-tracking branch 'upstream/branch-23.10' into branch-23.1…

e084d5d

…0-nx_cugraph_wheel

This comment was marked as duplicate.

Sign in to view

rlratzel marked this pull request as ready for review September 22, 2023 20:32

rlratzel requested a review from a team as a code owner September 22, 2023 20:32

rlratzel commented Sep 22, 2023

View reviewed changes

rlratzel requested review from vyasr and bdice September 22, 2023 20:50

Updates to use python package names where needed (- to _).

a662ec7

This comment was marked as duplicate.

Sign in to view

raydouglass approved these changes Sep 25, 2023

View reviewed changes

Merge remote-tracking branch 'upstream/branch-23.10' into branch-23.1…

8d7546e

…0-nx_cugraph_wheel

This comment was marked as duplicate.

Sign in to view

Removes FIXME that was addressed in a prior commit.

5adefe8

This comment was marked as duplicate.

Sign in to view

vyasr approved these changes Sep 26, 2023

View reviewed changes

Revert update of copyright date since no changes were made.

9f81547

This comment was marked as duplicate.

Sign in to view

bdice requested changes Sep 26, 2023

View reviewed changes

bdice approved these changes Sep 26, 2023

View reviewed changes

rapids-bot bot merged commit 5d2f548 into rapidsai:branch-23.10 Sep 27, 2023
67 checks passed

raydouglass mentioned this pull request Oct 2, 2023

Updates to 23.12 #3905

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates to build and test `nx-cugraph` wheel as part of CI and nightly workflows #3852

Updates to build and test `nx-cugraph` wheel as part of CI and nightly workflows #3852

rlratzel commented Sep 8, 2023 •

edited

Loading

This comment was marked as resolved.

rlratzel commented Sep 8, 2023 •

edited

Loading

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

rlratzel Sep 22, 2023

vyasr Sep 26, 2023

bdice Sep 26, 2023

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

vyasr left a comment

vyasr Sep 26, 2023

vyasr Sep 26, 2023

bdice Sep 26, 2023

bdice Sep 26, 2023

rlratzel Sep 26, 2023

bdice Sep 26, 2023

This comment was marked as duplicate.

bdice left a comment

bdice Sep 26, 2023

bdice Sep 26, 2023

rlratzel commented Sep 27, 2023

Updates to build and test nx-cugraph wheel as part of CI and nightly workflows #3852

Updates to build and test nx-cugraph wheel as part of CI and nightly workflows #3852

Conversation

rlratzel commented Sep 8, 2023 • edited Loading

This comment was marked as resolved.

rlratzel commented Sep 8, 2023 • edited Loading

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

vyasr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

bdice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlratzel commented Sep 27, 2023

Updates to build and test `nx-cugraph` wheel as part of CI and nightly workflows #3852

Updates to build and test `nx-cugraph` wheel as part of CI and nightly workflows #3852

rlratzel commented Sep 8, 2023 •

edited

Loading

rlratzel commented Sep 8, 2023 •

edited

Loading