Multi-GPU induced subgraph tests code #2602

yang-hu-nv · 2022-08-19T23:26:18Z

Define MG tests for induced subgraph. The test won't work until implementation is complete, which will be in a later PR. C API definition and C API tests will also be in a later PR.

Marked as breaking as the C++ API changes.

Closes #2531
Closes #2534

seunghwak · 2022-08-20T00:55:24Z

rerun tests

codecov-commenter · 2022-10-12T22:57:49Z

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.12@ed39ddb). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

@@               Coverage Diff               @@
##             branch-22.12    #2602   +/-   ##
===============================================
  Coverage                ?   60.44%           
===============================================
  Files                   ?      111           
  Lines                   ?     6515           
  Branches                ?        0           
===============================================
  Hits                    ?     3938           
  Misses                  ?     2577           
  Partials                ?        0

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

rlratzel · 2022-10-13T15:13:03Z

rerun tests

seunghwak · 2022-10-13T16:01:04Z

cpp/include/cugraph/graph_functions.hpp

- * num_subgraphs]). The elements of @p subgraph_vertices for each subgraph should be sorted in
- * ascending order and unique.
- * @param num_subgraphs Number of induced subgraphs to extract.
+ * @param subgraph_offsets Span pointing to subgraph vertex offsets


Shouldn't we better be little more specific here?

Maybe add something like the following.

@p subgraph_offsets and @p subgraph_vertices provide vertex sets (or local vertex sets in multi-GPU) for @p subgraph_offsets.size() - 1 subgraphs to extract. For the i'th subgraph to extract, one can extract the (local-)vertex set by accessing a subset of @p subgraph_vertices, where the range of the subset is [@p subgraph_offsetes[i], @p subgraph_offsets[i + 1]). In multi-GPU, the vertex set for each subgraph is distributed in multiple-GPUs and each GPU holds only the vertices that are local to the GPU.

Added the suggested text.

seunghwak · 2022-10-13T16:19:30Z

cpp/src/structure/induced_subgraph_impl.cuh

-      [subgraph_offsets, subgraph_vertices, num_subgraphs, edge_partition] __device__(auto i) {
+      [subgraph_offsets,
+       subgraph_vertices,
+       num_subgraphs = subgraph_offsets.size() - 1,


It seems like num_subgraphs is not used?

seunghwak · 2022-10-13T16:21:07Z

cpp/src/structure/induced_subgraph_impl.cuh

@@ -206,7 +208,7 @@ extract_induced_subgraphs(
      thrust::make_counting_iterator(num_aggregate_subgraph_vertices),
      [subgraph_offsets,
       subgraph_vertices,
-       num_subgraphs,
+       num_subgraphs = subgraph_offsets.size() - 1,


It seems like num_subgraphs is not used?

seunghwak · 2022-10-13T16:23:12Z

cpp/tests/structure/induced_subgraph_test.cpp

+      hr_clock.start();
+    }
+
+    // FIXME: turn-off do_expensive_check once verified.


I guess this code is now verified, so we can turn-off expensive check and delete this FIXME.

seunghwak · 2022-10-13T16:48:08Z

cpp/tests/structure/mg_induced_subgraph_test.cpp

+                                  h_cugraph_subgraph_edgelist_majors,
+                                  h_cugraph_subgraph_edgelist_minors,
+                                  h_cugraph_subgraph_edgelist_weights,
+                                  h_cugraph_subgraph_edge_offsets);


So, we typically compare the SG result with the MG result in multi-GPU testing (and compare with the reference code output in SG testing). If I am not mistaken, here, we are comparing with the reference code output in MG as well. Any motivation for this?

Too lazy to write two different comparison functions :-)

It felt like better code reuse just to validate against the reference implementation rather than write a second independent test.

So, the initial motivation is that, we may run with larger data on MG, and comparing with the SG result will allow testing with bigger data than a sequential reference implementation. When I test something on multi-GPUs, I sometimes override the rmat_scale to the largest that fit within a single GPU to validate the MG results against the SG.

Do you want to move away from this convention (and do the same for other tests as well) or may compare with the reference just for induced subgraph?

That is not something that I have been doing when testing MG, but I can see the value in that. I will look at recasting this test to compare against SG.

Latest push has refactored this.

…eference implementation

seunghwak · 2022-10-17T21:19:05Z

cpp/tests/utilities/test_utilities_impl.cuh

+template <typename vertex_t, typename edge_t, typename weight_t, bool store_transposed>
+std::tuple<cugraph::graph_t<vertex_t, edge_t, weight_t, store_transposed, false>,
+           std::optional<rmm::device_uvector<vertex_t>>>
+mg_graph_to_sg_graph(


I think this is a better mechanism than reading/creating a separate SG graph as we have been doing elsewhere (For tests with R-mat graphs, we rely on the fact that our pseduo random number generator is deterministic for a given seed, but we no longer need to rely on this).

Not a high priority thing, but we may better apply this mechanism in all the other MG C++ tests as well.

I was thinking this when I created it. Once we merge this PR I'll add an issue to the backlog to update the other tests.

BradReesWork · 2022-10-18T16:21:25Z

rerun tests

ChuckHastings · 2022-10-18T20:08:14Z

@gpucibot merge

fea_mg_induced_subgraph

9816785

yang-hu-nv requested review from a team as code owners August 19, 2022 23:26

seunghwak changed the title ~~[WIP][skip-ci] fea_mg_induced_subgraph~~ Multi-GPU induced subgraph tests code Aug 20, 2022

seunghwak added feature request New feature or request 2 - In Progress non-breaking Non-breaking change labels Aug 20, 2022

BradReesWork added this to the 22.10 milestone Aug 29, 2022

BradReesWork added improvement Improvement / enhancement to an existing function and removed feature request New feature or request 2 - In Progress labels Aug 29, 2022

ChuckHastings added 2 commits August 30, 2022 14:50

Merge branch 'branch-22.10' into fea_mg_induced_subgraph

a85a5ff

a few new updates

dc14c99

BradReesWork requested review from seunghwak and ChuckHastings September 20, 2022 16:49

BradReesWork assigned ChuckHastings Sep 20, 2022

BradReesWork modified the milestones: 22.10, 22.12 Sep 29, 2022

BradReesWork changed the base branch from branch-22.10 to branch-22.12 September 29, 2022 17:14

ChuckHastings added 5 commits October 5, 2022 14:35

Merge branch 'branch-22.12' into fea_mg_induced_subgraph

a2a3c0b

finished refactoring/implementation of induced subgraph tests

96a5365

move induced subgraph test code to structure directory

a576c45

finish moving induced subgraph from community to structure

87e73b6

fix copyright on induced_subgraph_test

2814a5e

ChuckHastings added 3 - Ready for Review breaking Breaking change and removed non-breaking Non-breaking change labels Oct 12, 2022

fix clang-format issues

0990e35

ChuckHastings removed their request for review October 12, 2022 18:28

seunghwak reviewed Oct 13, 2022

View reviewed changes

ChuckHastings added 7 commits October 13, 2022 16:34

fix a few things from PR review and testing failures

8678064

refactor mg induced subgraph test to compare with sg instead of CPU r…

b03f5cc

…eference implementation

a few minor tweaks

25d39c9

fix clang-format issues

6afadcb

Merge branch 'branch-22.12' into fea_mg_induced_subgraph

3cd7c73

Seunghwa and I both added function, can only be there once

d232521

address remaining comments from initial PR review

36f7e68

seunghwak approved these changes Oct 17, 2022

View reviewed changes

bad parameter in call to induced subgraphs

4ff0d61

rapids-bot bot merged commit 856e3ba into rapidsai:branch-22.12 Oct 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi-GPU induced subgraph tests code #2602

Multi-GPU induced subgraph tests code #2602

yang-hu-nv commented Aug 19, 2022 •

edited by ChuckHastings

Loading

seunghwak commented Aug 20, 2022

codecov-commenter commented Oct 12, 2022 •

edited

Loading

rlratzel commented Oct 13, 2022

seunghwak Oct 13, 2022

ChuckHastings Oct 17, 2022

seunghwak Oct 13, 2022

ChuckHastings Oct 13, 2022

seunghwak Oct 13, 2022

ChuckHastings Oct 13, 2022

seunghwak Oct 13, 2022

ChuckHastings Oct 17, 2022

seunghwak Oct 13, 2022

ChuckHastings Oct 13, 2022

seunghwak Oct 13, 2022

ChuckHastings Oct 13, 2022

ChuckHastings Oct 17, 2022

seunghwak Oct 17, 2022

ChuckHastings Oct 17, 2022

BradReesWork commented Oct 18, 2022

ChuckHastings commented Oct 18, 2022

Multi-GPU induced subgraph tests code #2602

Multi-GPU induced subgraph tests code #2602

Conversation

yang-hu-nv commented Aug 19, 2022 • edited by ChuckHastings Loading

seunghwak commented Aug 20, 2022

codecov-commenter commented Oct 12, 2022 • edited Loading

Codecov Report

rlratzel commented Oct 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BradReesWork commented Oct 18, 2022

ChuckHastings commented Oct 18, 2022

yang-hu-nv commented Aug 19, 2022 •

edited by ChuckHastings

Loading

codecov-commenter commented Oct 12, 2022 •

edited

Loading