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

Raft Handle Updates to cuGraph #1894

Merged
merged 15 commits into from
Dec 14, 2021
Merged

Raft Handle Updates to cuGraph #1894

merged 15 commits into from
Dec 14, 2021

Conversation

divyegala
Copy link
Member

Depends on rapidsai/raft#291

@divyegala divyegala requested review from a team as code owners October 18, 2021 22:21
@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2021 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.02@b7d63bb). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-22.02    #1894   +/-   ##
===============================================
  Coverage                ?   70.50%           
===============================================
  Files                   ?      142           
  Lines                   ?     8880           
  Branches                ?        0           
===============================================
  Hits                    ?     6261           
  Misses                  ?     2619           
  Partials                ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7d63bb...66bac49. Read the comment docs.

@BradReesWork BradReesWork added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 19, 2021
@BradReesWork BradReesWork added this to the 21.12 milestone Oct 19, 2021
@BradReesWork
Copy link
Member

rerun tests

seunghwak
seunghwak reviewed Oct 19, 2021
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.

Thanks for making this update, looks good to me except for the question below.

cpp/cmake/thirdparty/get_raft.cmake Outdated Show resolved Hide resolved
ChuckHastings
ChuckHastings reviewed Oct 19, 2021
View reviewed changes
Copy link
Collaborator

@ChuckHastings ChuckHastings left a comment

Choose a reason for hiding this comment

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

Looks good. Will like to review again when the final changes are made and everything builds.

@rlratzel
Copy link
Contributor

rerun tests

1 similar comment
@ajschmidt8
Copy link
Member

rerun tests

@divyegala divyegala requested a review from a team as a code owner October 28, 2021 21:32
ajschmidt8
ajschmidt8 approved these changes Oct 28, 2021
View reviewed changes
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

seunghwak
seunghwak reviewed Nov 2, 2021
View reviewed changes
cpp/cmake/thirdparty/get_raft.cmake Outdated
FORK rapidsai
PINNED_TAG branch-${CUGRAPH_BRANCH_VERSION_raft}
FORK divyegala
PINNED_TAG imp-21.10-handle_stream
Copy link
Contributor

Choose a reason for hiding this comment

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

I am waiting for this PR to pull raft from the main repo to approve this.

@BradReesWork BradReesWork modified the milestones: 21.12, 22.02 Nov 15, 2021
@divyegala divyegala changed the base branch from branch-21.12 to branch-22.02 November 16, 2021 19:50
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@divyegala divyegala requested a review from a team as a code owner November 17, 2021 22:48
@ChuckHastings
Copy link
Collaborator

This PR includes merges that aren't in 22.02 yet (broken auto merge). I will finish reviewing once the auto merge has brought things in sync.

@rlratzel
Copy link
Contributor

rerun tests

Rerunning to get latest updates that should fix build problem.

ChuckHastings
ChuckHastings approved these changes Nov 22, 2021
View reviewed changes
Copy link
Collaborator

@ChuckHastings ChuckHastings left a comment

Choose a reason for hiding this comment

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

Still needs to be updated after the dependency is merged to point to the real raft.

Otherwise this looks good. It would be good to get this merged soon, it's really ugly to review with all of the intervening commits.

@rlratzel
Copy link
Contributor

This PR will need #1968 merged in to branch 22.02.

@rlratzel
Copy link
Contributor

rerun tests

PR #1968 was merged in to 21.12 and the forward merge to 22.02 has finished.

@rlratzel
Copy link
Contributor

rlratzel commented Dec 6, 2021

rerun tests

The latest cudf should fix the seg fault in the failing unit tests.

@rlratzel rlratzel mentioned this pull request Dec 8, 2021
Closed
@rlratzel
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 7197258 into rapidsai:branch-22.02 Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seunghwak seunghwak seunghwak left review comments

@rlratzel rlratzel rlratzel approved these changes

@ChuckHastings ChuckHastings ChuckHastings approved these changes

@BradReesWork BradReesWork BradReesWork approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

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

Successfully merging this pull request may close these issues.
7 participants
@divyegala @codecov-commenter @BradReesWork @rlratzel @ajschmidt8 @ChuckHastings @seunghwak