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 Refactored cython graph factory code to scale to additional data types #1178

Conversation

rlratzel
Copy link
Contributor

@rlratzel rlratzel commented Oct 1, 2020

This code originated from @ChuckHastings 's work here: rlratzel#3 (commit: 785ca49)

This update changes the implementation of the cython graph factory code and corresponding container type to allow for easier scaling to different data types. The changes are mostly insulated from clients (the cython wrapper code for each algo) and the client APIs remain almost completely unchanged.

This update is needed to support int64 types needed for graphs with >2B vertices.

rlratzel and others added 30 commits September 16, 2020 19:38
…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).
… ctor. Now having a problem finding the right subcommunicator.
…y subcomms not being initialized: "address not mapped to object at address (nil)"
… (using num edges for partition instead of global for edgelist) and for debugging (print statments).
@rlratzel rlratzel requested review from a team as code owners October 1, 2020 03:06
@rlratzel rlratzel self-assigned this Oct 1, 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 Oct 1, 2020
@rlratzel rlratzel changed the title [WIP][skip-ci] ENH Refactored cython graph factory code to scale to additional data types [REVIEW] ENH Refactored cython graph factory code to scale to additional data types Oct 2, 2020
@rlratzel
Copy link
Contributor Author

rlratzel commented Oct 2, 2020

rerun tests

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2020

Codecov Report

Merging #1178 into branch-0.16 will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.16    #1178      +/-   ##
===============================================
- Coverage        57.41%   57.39%   -0.03%     
===============================================
  Files               61       61              
  Lines             2482     2483       +1     
===============================================
  Hits              1425     1425              
- Misses            1057     1058       +1     
Impacted Files Coverage Δ
python/cugraph/dask/community/louvain.py 31.81% <0.00%> (-1.52%) ⬇️

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 765d776...5adb743. Read the comment docs.

cpp/include/utilities/cython.hpp Show resolved Hide resolved
cpp/include/utilities/cython.hpp Show resolved Hide resolved
cpp/src/experimental/graph.cu Outdated Show resolved Hide resolved
cpp/src/utilities/cython.cu Outdated Show resolved Hide resolved
…on wrapper and removed unnecessary vertex_t/edge_t int64,int32 combinations.
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 the update!!!

@BradReesWork BradReesWork merged commit 60b9b85 into rapidsai:branch-0.16 Oct 2, 2020
@rlratzel rlratzel deleted the branch-0.16-cythonfactoryrefactoring 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.

8 participants