-
Notifications
You must be signed in to change notification settings - Fork 304
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
Modify MNMG louvain to support an empty vertex partition #1744
Modify MNMG louvain to support an empty vertex partition #1744
Conversation
Codecov Report
@@ Coverage Diff @@
## branch-21.10 #1744 +/- ##
===============================================
Coverage ? 59.75%
===============================================
Files ? 77
Lines ? 3523
Branches ? 0
===============================================
Hits ? 2105
Misses ? 1418
Partials ? 0 Continue to review full report at Codecov.
|
vertex_t* clustering) | ||
{ | ||
if (graph_view.get_number_of_local_vertices() > 0) | ||
CUGRAPH_EXPECTS(clustering != nullptr, "Invalid input argument: clustering is null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really an invalid use? I assume it is valid (even though hugely inefficient) to ask cuGraph to compute Louvain for a tiny graph using many GPUs (e.g. # GPUs > # vertices).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the number of local vertices is 0 then the clustering
pointer is never referenced, nothing is required to be written.
The issue that triggered this is that if the graph is poorly distributed (karate over 8 GPUs apparently ends up with at least 1 cluster which has no vertices assigned to it) then the python code creates a cudf column with 0 entries. This results in one of the calls using clustering == nullptr
, which was failing. This check will allow clustering
to be nullptr
if there are no vertices assigned to this GPU. If there are vertices assigned to this GPU it will fail if clustering
is nullptr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... I see. I misunderstood the code and thanks for the explanation.
@gpucibot merge |
Discovered in MNMG testing.
Closes #1713