Address some MNMG issues in cython.cu #2224
Conversation
…e in renumber_helper
ChuckHastings
added
3 - Ready for Review
bug
| @@ -594,9 +594,10 @@ std::unique_ptr<major_minor_weights_t<vertex_t, edge_t, weight_t>> call_shuffle( | |||
| edgelist_major_vertices, // [IN / OUT]: groupby_gpu_id_and_shuffle_values() sorts in-place | |||
| vertex_t* edgelist_minor_vertices, // [IN / OUT] | |||
| weight_t* edgelist_weights, // [IN / OUT] | |||
| edge_t num_edgelist_edges); | |||
| edge_t num_edgelist_edges, | |||
| bool is_weighted); | |||
There was a problem hiding this comment.
Just FYI, we're using std::optional elsewhere to address this instead of adding an additional flag. Not sure it is worth the effort in applying the same strategy here as this code will become obsolete once we switch to the C-API based approach.
There was a problem hiding this comment.
And yeah... this is cython, so we may not be able to use std optional here.
Codecov Report
@@ Coverage Diff @@
## branch-22.06 #2224 +/- ##
================================================
+ Coverage 70.82% 70.86% +0.04%
================================================
Files 170 170
Lines 11036 11036
================================================
+ Hits 7816 7821 +5
+ Misses 3220 3215 -5
Continue to review full report at Codecov.
|
| @@ -65,7 +65,8 @@ cdef renumber_helper(shuffled_vertices_t* ptr_maj_min_w, vertex_t, weights): | |||
| move(pair_s_weights.first), weight_t, "shuffled_weights") | |||
| if shuffled_weights_series is None: | |||
| shuffled_df['value']= cudf.Series(dtype=weight_t) | |||
| shuffled_df['value']= shuffled_weights_series | |||
| else: | |||
| shuffled_df['value']= shuffled_weights_series | |||
There was a problem hiding this comment.
I believe we should do the same for shuffled_minor_series and shuffled_major_series
There was a problem hiding this comment.
Good catch. Wasn't causing an error, but the code is, in fact, incorrect. Updated in the last push.
|
@gpucibot merge |
Closes #2217
The issue identifies some issues if the partitioning ends up with some empty partitions. There were two issues here:
call_shufflemethod determines whether a graph has weights by comparing the weight pointer tonullptr. But if an MNMG partition is empty then the weight pointer will benullptreven if the graph has weights. Added a parameter to identify if the graph is weighted.renumber_helpermethod was missing an else and was overwriting the correct value with an invalid value if the return from shuffling on a gpu had no entries.These are corrected in the PR.