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

[BUG] Fix Incorrect Edge Index, Directory Selection in cuGraph-PyG Loader #3978

Merged
merged 25 commits into from
Nov 17, 2023
Merged

[BUG] Fix Incorrect Edge Index, Directory Selection in cuGraph-PyG Loader #3978

merged 25 commits into from
Nov 17, 2023

Conversation

alexbarghi-nv
Copy link
Member

@alexbarghi-nv alexbarghi-nv commented Nov 6, 2023 •
edited
Loading

Fixes three major bugs:

  1. Edge index is set to [dst, dst] instead of [dst, src] in some cases
  2. The sample directory is always set to a new temporary directory rather than the path given
  3. The version of pylibcugraphops in meta.yaml is wrong and causes the wrong packages to be resolved

This PR also simplifies ci/test_python.sh by doing only a single conda install when creating the test_cugraph_pyg environment.

Closes #3959

@alexbarghi-nv alexbarghi-nv self-assigned this Nov 6, 2023
@alexbarghi-nv alexbarghi-nv added non-breaking Non-breaking change CRITICAL BUG! BUG that needs to be FIX NOW !!!! bug Something isn't working labels Nov 6, 2023
@alexbarghi-nv alexbarghi-nv marked this pull request as ready for review November 6, 2023 18:43
@alexbarghi-nv alexbarghi-nv requested a review from a team as a code owner November 6, 2023 18:43
@alexbarghi-nv alexbarghi-nv requested a review from a team as a code owner November 7, 2023 16:56
@alexbarghi-nv alexbarghi-nv mentioned this pull request Nov 7, 2023
Merged
tingyu66
tingyu66 reviewed Nov 9, 2023
View reviewed changes
Copy link
Member

@tingyu66 tingyu66 left a comment

Choose a reason for hiding this comment

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

Left some comments. LGTM otherwise.

python/cugraph-pyg/cugraph_pyg/loader/cugraph_node_loader.py Outdated Show resolved Hide resolved
python/cugraph-pyg/cugraph_pyg/tests/test_cugraph_loader.py Outdated Show resolved Hide resolved
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Nov 10, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@alexbarghi-nv
Copy link
Member Author

/ok to test

@alexbarghi-nv
Copy link
Member Author

/ok to test

@alexbarghi-nv alexbarghi-nv added this to the 23.12 milestone Nov 10, 2023
@alexbarghi-nv
Copy link
Member Author

/ok to test

@alexbarghi-nv alexbarghi-nv requested a review from a team as a code owner November 12, 2023 17:19
@alexbarghi-nv
Copy link
Member Author

/ok to test

@alexbarghi-nv
Copy link
Member Author

/ok to test

@alexbarghi-nv
Copy link
Member Author

/ok to test

@alexbarghi-nv
Copy link
Member Author

/merge

1 similar comment
@BradReesWork
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 119816c into rapidsai:branch-23.12 Nov 17, 2023
74 checks passed
@alexbarghi-nv alexbarghi-nv deleted the fix-loader-bugs branch November 17, 2023 14:16
rapids-bot bot pushed a commit that referenced this pull request Nov 20, 2023
@alexbarghi-nv
[BUG] Fix Graph Construction From Pandas in cuGraph-PyG (#3985)
0f28b2e 
The current graph construction creates a single pandas dataframe, which for larger datasets (i.e. ogbn-papers100M) cannot be serialized.  This PR resolves this by breaking up the dataframe into scattered numpy arrays that are then reassembled.

Merge after #3978

Authors:
  - Alex Barghi (https://github.com/alexbarghi-nv)
  - Naim (https://github.com/naimnv)

Approvers:
  - Vibhu Jawa (https://github.com/VibhuJawa)
  - Brad Rees (https://github.com/BradReesWork)
  - Tingyu Wang (https://github.com/tingyu66)

URL: #3985
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tingyu66 tingyu66 tingyu66 left review comments

@BradReesWork BradReesWork BradReesWork approved these changes

@raydouglass raydouglass raydouglass approved these changes

Assignees

@alexbarghi-nv alexbarghi-nv

Labels
bug Something isn't working CRITICAL BUG! BUG that needs to be FIX NOW !!!! non-breaking Non-breaking change
Projects
None yet
Milestone
23.12
Development

Successfully merging this pull request may close these issues.

[BUG] cuGraph-PyG BulkSampleLoader Doesn't Handle Directories Correctly
5 participants
@alexbarghi-nv @BradReesWork @raydouglass @tingyu66 @naimnv