Fix MG Louvain test compile errors #1797
Conversation
seunghwak
added
3 - Ready for Review
bug
| @@ -85,8 +85,7 @@ compressed_sparse_to_edgelist(edge_t const* compressed_sparse_offsets, | |||
| // FIXME: this is highly inefficient for very high-degree vertices, for better performance, we can | |||
| // fill high-degree vertices using one CUDA block per vertex, mid-degree vertices using one CUDA | |||
| // warp per vertex, and low-degree vertices using one CUDA thread per block | |||
| auto execution_policy = handle.get_thrust_policy(); | |||
| thrust::for_each(execution_policy, | |||
| thrust::for_each(rmm::exec_policy(stream), | |||
There was a problem hiding this comment.
I imagine this was missed because CI doesn't compile MG analytics.
Would it be better to change the function signature to pass in handle? That would make it more consistent with the rest of the code base.
There was a problem hiding this comment.
Would it be better to change the function signature to pass in
handle? That would make it more consistent with the rest of the code base.
Ideally, this code should be deleted as this file is pretty much copy-and-paste (plus little modification) of https://github.com/rapidsai/cugraph/blob/branch-21.10/cpp/src/structure/coarsen_graph.cu
The reason this function is designed to take stream instead of handle is to facilitate multi-stream execution. Different matrix partitions can be concurrently executed in multiple CUDA streams and such functions take stream instead of handle (another approach might be passing handle + stream index if we see multiple cases using other handle features).
|
rerun tests |
Codecov Report
@@ Coverage Diff @@
## branch-21.10 #1797 +/- ##
=================================================
+ Coverage 59.85% 71.52% +11.67%
=================================================
Files 77 134 +57
Lines 3547 8126 +4579
=================================================
+ Hits 2123 5812 +3689
- Misses 1424 2314 +890
Continue to review full report at Codecov.
|
|
@gpucibot merge |
Fix MG Louvain test compile errors after recent PR merges involving thrust execution policies.