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

Update cugraph for compatibility with the latest cuco #4111

Merged
merged 16 commits into from
Feb 17, 2024
Merged

Update cugraph for compatibility with the latest cuco #4111

merged 16 commits into from
Feb 17, 2024

Conversation

PointKernel
Copy link
Member

@PointKernel PointKernel commented Jan 23, 2024 •
edited
Loading

This PR updates cugraph to make it compatible with the latest cuco.

Depends on rapidsai/rapids-cmake#526

CMake changes will be reverted once rapidsai/rapids-cmake#526 is merged.

@PointKernel PointKernel mentioned this pull request Jan 23, 2024
Merged
10 tasks
@BradReesWork BradReesWork added this to the 24.04 milestone Jan 25, 2024
@PointKernel PointKernel changed the title Fix cuco merge conflicts Update cugraph for compatibility with the latest cuco Jan 30, 2024
@PointKernel PointKernel marked this pull request as ready for review February 6, 2024 19:16
@PointKernel PointKernel requested a review from a team as a code owner February 6, 2024 19:16
seunghwak
seunghwak approved these changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

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

Besides the minor comments about naming, looks good to me.

cpp/src/prims/key_store.cuh Outdated Show resolved Hide resolved
@seunghwak seunghwak added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 9, 2024
@naimnv
Copy link
Contributor

naimnv commented Feb 9, 2024

/ok to test

@BradReesWork
Copy link
Member

/ok to test

@BradReesWork
Copy link
Member

/ok to test

@PointKernel
Copy link
Member Author

PointKernel commented Feb 13, 2024 •
edited
Loading

The pip devcontainer CI failed because cugraph fetches the proper cuco version but it's still using the "main" cudf branch which doesn't include the changes in rapidsai/cudf#14849.

Not sure what's the best way to fix this. Solved by customizing cudf version in 6b42728

@PointKernel PointKernel requested a review from a team as a code owner February 13, 2024 18:30
@github-actions github-actions bot added the CMake label Feb 13, 2024
@PointKernel PointKernel changed the title Update cugraph for compatibility with the latest cuco [DO NOT MERGE] Update cugraph for compatibility with the latest cuco Feb 13, 2024
rapids-bot bot pushed a commit to rapidsai/rapids-cmake that referenced this pull request Feb 15, 2024
@PointKernel
Fetch the latest cuco and remove outdated patches (#526)
e3d1dfc 
This PR bumps to the latest cuco version which removes experimental namespace and includes multiple bug fixes.

CI must pass on these PRs before we can merge this change:
- [ ] rapidsai/cudf#14849
- [ ] rapidsai/cugraph#4111
- [ ] rapidsai/raft#2118

Authors:
  - Yunsong Wang (https://github.com/PointKernel)
  - Robert Maynard (https://github.com/robertmaynard)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Bradley Dice (https://github.com/bdice)

URL: #526
@github-actions github-actions bot removed the CMake label Feb 15, 2024
@PointKernel PointKernel changed the title [DO NOT MERGE] Update cugraph for compatibility with the latest cuco Update cugraph for compatibility with the latest cuco Feb 15, 2024
@PointKernel
Copy link
Member Author

This is ready to go once CI passes. Note the CI will be blocked until this PR gets merged.

Thanks!

@PointKernel
Copy link
Member Author

Can someone please rerun the failed CI? They should pass this time.

@ChuckHastings
Copy link
Collaborator

Can someone please rerun the failed CI? They should pass this time.

Still failing. Have the artifacts all been updated?

@PointKernel
Copy link
Member Author

Can someone please rerun the failed CI? They should pass this time.

Still failing. Have the artifacts all been updated?

I think it's because cugraph is using the cuco version from raft and the upstream raft work rapidsai/raft#2118 hasn't been merged yet.

@PointKernel
Copy link
Member Author

@ChuckHastings could you please give it another try? rapidsai/raft#2118 just gets merged.

@jakirkham jakirkham mentioned this pull request Feb 16, 2024
Merged
@jakirkham
Copy link
Member

One of the devcontainer builds (like still getting old packages due to the cuco errors in the log). Restarted that job

@jakirkham
Copy link
Member

This is now passing! 🎉

@jakirkham
Copy link
Member

Going to go ahead and merge as this was already approved above. We were just waiting on CI. Hope that is ok

@jakirkham
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit f0388bc into rapidsai:branch-24.04 Feb 17, 2024
119 checks passed
@PointKernel PointKernel deleted the fix-cuco-merge-conflicts branch February 17, 2024 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seunghwak seunghwak seunghwak approved these changes

@ChuckHastings ChuckHastings ChuckHastings approved these changes

Assignees
No one assigned
Labels
cuGraph improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Milestone
24.04
Development

Successfully merging this pull request may close these issues.
7 participants
@PointKernel @naimnv @BradReesWork @ChuckHastings @jakirkham @seunghwak @vyasr