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

[REVIEW] ENH Integrate 2D shuffling and latest Louvain API #1163

Merged

Conversation

rlratzel
Copy link
Contributor

@rlratzel rlratzel commented Sep 28, 2020

This PR integrates the latest updates to both 2D shuffling and the Louvain C++ work to allow for a complete e2e call from the user API into the Louvain MNMG C++.

closes #1050

…ating legacy types and switch statements for it to factory function.
…graphfactoryupdate, and safety commit: have graph_t Louvain version building and test running (still failing).
@rlratzel rlratzel requested review from a team as code owners September 28, 2020 14:41
@rlratzel rlratzel self-assigned this Sep 28, 2020
@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@BradReesWork BradReesWork added this to the 0.16 milestone Sep 28, 2020
Iroy30 and others added 2 commits September 30, 2020 12:19
…by default, with an option to enable "option 2", temporarily enabled graph expensive checks for debugging.
…one bug in cython code, added copyright to shuffle.py, changed how ranks are retrieved from the raft handle.
@Iroy30 Iroy30 self-requested a review September 30, 2020 21:30
Copy link
Contributor

@Iroy30 Iroy30 left a comment

Choose a reason for hiding this comment

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

lgtm

@rlratzel rlratzel changed the title [WIP][skip-ci] Integrate 2D shuffling and latest Louvain API [REVIEW] ENH Integrate 2D shuffling and latest Louvain API Sep 30, 2020
@rlratzel
Copy link
Contributor Author

rerun tests

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.

Remaining things will be addressed in follow on PR. This one is needed to be merged for easier integration with other work.

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2020

Codecov Report

Merging #1163 into branch-0.16 will decrease coverage by 0.48%.
The diff coverage is 12.50%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.16    #1163      +/-   ##
===============================================
- Coverage        57.97%   57.49%   -0.48%     
===============================================
  Files               61       61              
  Lines             2458     2482      +24     
===============================================
+ Hits              1425     1427       +2     
- Misses            1033     1055      +22     
Impacted Files Coverage Δ
python/cugraph/structure/shuffle.py 10.14% <3.84%> (-2.82%) ⬇️
python/cugraph/dask/common/input_utils.py 21.92% <14.28%> (-0.51%) ⬇️
python/cugraph/dask/community/louvain.py 33.33% <42.85%> (+1.75%) ⬆️

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 1f58e1a...da6a50c. Read the comment docs.

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.

OK, I guess these are my last complaints :-)

cpp/src/utilities/cython.cpp Show resolved Hide resolved
cpp/src/utilities/cython.cpp Show resolved Hide resolved
@BradReesWork BradReesWork merged commit 891bf43 into rapidsai:branch-0.16 Oct 1, 2020
@rlratzel rlratzel deleted the branch-0.16-graphfactoryupdate branch December 9, 2020 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH] Dask to 2D partitioin pipeline
8 participants