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

[BUG] Restore the original default order of CSR, which does not reverse edges in cuGraph-PyG #3980

Merged

Conversation

alexbarghi-nv
Copy link
Member

@alexbarghi-nv alexbarghi-nv commented Nov 6, 2023

OGB and many other datasets express the edge index in CSR order, but in release 23.10, the default order was changed to CSC, which broke downstream workflows. This PR restores the original default, which should be acceptable for the most common datasets, and familiar to most PyG users.

Closes #3969

@alexbarghi-nv alexbarghi-nv added bug Something isn't working breaking Breaking change CRITICAL BUG! BUG that needs to be FIX NOW !!!! labels Nov 6, 2023
@alexbarghi-nv alexbarghi-nv added this to the 23.12 milestone Nov 6, 2023
@alexbarghi-nv alexbarghi-nv self-assigned this Nov 6, 2023
@alexbarghi-nv alexbarghi-nv requested a review from a team as a code owner November 6, 2023 19:44
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.

LGTM.

Just to clarify, "CSR" in this context translates into: using destination nodes (in the COO) as seeds during sampling, right?

@alexbarghi-nv
Copy link
Member Author

LGTM.

Just to clarify, "CSR" in this context translates into: using destination nodes (in the COO) as seeds during sampling, right?

Yes

Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

The change looks reasonable to me (disclaimer: I'm not familiar with this code), but I'm surprised there aren't any tests that need to be changed too.

@rlratzel
Copy link
Contributor

rlratzel commented Nov 7, 2023

/merge

@rapids-bot rapids-bot bot merged commit a3cda5d into rapidsai:branch-23.12 Nov 7, 2023
71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change bug Something isn't working CRITICAL BUG! BUG that needs to be FIX NOW !!!!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] CSC Order Causes Issues with OGB Datasets
3 participants