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

Check and correct if necessary code broken from cudf unique and drop_duplicates behavior change #3615

Closed
rlratzel opened this issue May 25, 2023 · 1 comment · Fixed by #3616
Closed

Check and correct if necessary code broken from cudf unique and drop_duplicates behavior change #3615

rlratzel opened this issue May 25, 2023 · 1 comment · Fixed by #3616
Assignees
@alexbarghi-nv
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Comments

@rlratzel
Copy link
Contributor

rlratzel commented May 25, 2023 •
edited
Loading

From comment here

cuDF has changed the behavior of unique and drop_duplicates . If cuGraph is relying on unique() calls from cuDF to give sorted results, they may need to be replaced by unique().sort_values(). This change caused test failures for cuML (resolved by this PR) and I suspect cuGraph may have some of the same problems -- but may not be caught by tests except this one failure in cugraph-pyg. It may be worthwhile for someone to review uses of unique / drop_duplicates in cuGraph for any issues that tests might not be catching.

Specific ones that jump out as potentially hazardous:

cc @bdice
@rlratzel rlratzel added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 25, 2023
@rlratzel rlratzel assigned rlratzel and alexbarghi-nv and unassigned rlratzel May 25, 2023
@rlratzel
Copy link
Contributor Author

Here's a list of search results.

@alexbarghi-nv alexbarghi-nv mentioned this issue May 25, 2023
Merged
rapids-bot bot pushed a commit that referenced this issue May 27, 2023
@alexbarghi-nv
[BUG] Fix Calls to cudf.DataFrame/Series.unique that relied on old be…
1e6da2b 
…havior (#3616)

Updates the code that relied on `unique()` to return values in sorted order by explicitly sorting the unique values.

Closes #3615

Authors:
  - Alex Barghi (https://github.com/alexbarghi-nv)
  - Brad Rees (https://github.com/BradReesWork)

Approvers:
  - Rick Ratzel (https://github.com/rlratzel)
  - Vibhu Jawa (https://github.com/VibhuJawa)

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

@alexbarghi-nv alexbarghi-nv

Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[BUG] Fix Calls to cudf.DataFrame/Series.unique that relied on old behavior alexbarghi-nv/cugraph
2 participants
@rlratzel @alexbarghi-nv