Heterogeneous renumbering implementation #4602

seunghwak · 2024-08-08T00:38:31Z

This PR implements heterogeneous renumbering for GNN.

In addition,

Update the existing (homogeneous) sampling post processing function test file extension from .cu to .cpp.
Remove the unused renumber_sampled_edgelist function (breaking because this function is removed)
Add a stride_fill utility function (thrust wrapper)
Add test utility functions to generate edge types & IDs.

Closes #4412

…to fea_heterogeneous_renumbering_impl

ChuckHastings

A few minor things. Looks good.

ChuckHastings · 2024-08-13T18:42:32Z

cpp/tests/CMakeLists.txt

@@ -576,7 +582,8 @@ if(BUILD_CUGRAPH_MG_TESTS)
    ###############################################################################################
    # - MG BETWEENNESS CENTRALITY tests -----------------------------------------------------------
    ConfigureTestMG(MG_BETWEENNESS_CENTRALITY_TEST centrality/mg_betweenness_centrality_test.cpp)
-    ConfigureTestMG(MG_EDGE_BETWEENNESS_CENTRALITY_TEST centrality/mg_edge_betweenness_centrality_test.cpp)
+    ConfigureTestMG(MG_EDGE_BETWEENNESS_CENTRALITY_TEST


This seems like an odd edit.

This exceeded the line length limit :-) Should we allow long lines in CMakeLists.txt?

I didn't realize we had one.

I guess we don't have one officially, but I implicitly consider the end of ############################################################################################### as the line limit.

ChuckHastings · 2024-08-13T18:45:12Z

cpp/tests/sampling/sampling_heterogeneous_post_processing_test.cpp

@@ -0,0 +1,828 @@
+/*
+ * Copyright (c) 2022-2024, NVIDIA CORPORATION.


…to fea_heterogeneous_renumbering_impl

alexbarghi-nv · 2024-08-22T01:48:41Z

cpp/include/cugraph/sampling_functions.hpp

@@ -510,6 +514,11 @@ renumber_and_sort_sampled_edgelist(
 * true.
 * 2. Edges in each label are sorted independently if @p edgelist_label_offsets.has_value() is true.
 *
+ * This function assumes that there is a single edge source vertex type and a single edge
+ * destination vertex type for each edge. If @p edgelist_edge_types.has_value() is false (i.e. there
+ * is only one edge type), there shouldb be only one edge source vertex type and only one edge


typo here

Suggested change

* is only one edge type), there shouldb be only one edge source vertex type and only one edge

* is only one edge type), there should be only one edge source vertex type and only one edge

alexbarghi-nv

Looks good. I assume you're delegating updating the C API sampling function to properly handle heterogeneous renumbering to me.

ChuckHastings · 2024-08-22T16:18:31Z

cpp/src/sampling/sampling_post_processing_impl.cuh

      "Invalid input arguments: num_labels should be 0 if the input edge list is empty.");
    CUGRAPH_EXPECTS(
-      "num_hops == 0",
+      num_hops == 0,


This fixed a bug in the C++ layer (the old check always passed because of the quotes), but adding this check causes a failure in a python test.

Is there a reason to fail with an exception if num_hops is 0? If the input edge list is empty, couldn't we just return an empty result? I'm unfamiliar with the logic.

This is failing in CI, we should either modify the python code or remove this check.

Yeah... I guess you're right. We can check only for nun_labels == 0 for an empty edge list and allow num_hops & num_vertex_types to have a non zero value.

ChuckHastings · 2024-08-22T16:24:54Z

Looks good. I assume you're delegating updating the C API sampling function to properly handle heterogeneous renumbering to me.

Do you have time to do that @alexbarghi-nv ? We can do that as a separate PR if you are busy with other things. Would you want to always use this new heterogeneous renumbering logic, add an option for it, or detect it in some other way?

alexbarghi-nv · 2024-08-22T16:27:16Z

Looks good. I assume you're delegating updating the C API sampling function to properly handle heterogeneous renumbering to me.

Do you have time to do that @alexbarghi-nv ? We can do that as a separate PR if you are busy with other things. Would you want to always use this new heterogeneous renumbering logic, add an option for it, or detect it in some other way?

We only want to use it for heterogeneous graphs. I think we can just add an optional parameter for the edge type mappings, and if they are present, then the C API sampling function will call the heterogeneous renumbering with the mappings instead of the homogeneous renumbering.

alexbarghi-nv · 2024-08-22T17:37:36Z

@seunghwak it looks like your PR changed the way we're handling empty start lists which is causing CI to fail:

FAILED tests/sampling/test_uniform_neighbor_sample.py::test_uniform_neighbor_sample_simple[graph_file:email_Eu_core-directed:True-with_replacement:False-indices_type:int32] - RuntimeError: non-success value returned from cugraph_uniform_neighbor_sample: CUGRAPH_UNKNOWN_ERROR cuGraph failure at file=/opt/conda/conda-bld/work/cpp/src/sampling/sampling_post_processing_impl.cuh line=235: Invalid input arguments: num_labels should be 0 if the input edge list is empty.

seunghwak · 2024-08-22T19:33:34Z

@seunghwak it looks like your PR changed the way we're handling empty start lists which is causing CI to fail:

FAILED tests/sampling/test_uniform_neighbor_sample.py::test_uniform_neighbor_sample_simple[graph_file:email_Eu_core-directed:True-with_replacement:False-indices_type:int32] - RuntimeError: non-success value returned from cugraph_uniform_neighbor_sample: CUGRAPH_UNKNOWN_ERROR cuGraph failure at file=/opt/conda/conda-bld/work/cpp/src/sampling/sampling_post_processing_impl.cuh line=235: Invalid input arguments: num_labels should be 0 if the input edge list is empty.

@ChuckHastings I assume this is also due to the fix in "num_hops == 0," right?

…to fea_heterogeneous_renumbering_impl

ChuckHastings · 2024-09-03T16:07:08Z

/merge

seunghwak added 19 commits July 30, 2024 15:25

update the API & documentation

f82dee3

remove unused code

2d8baa2

update heterogeneous renumbering API

f400c24

implemeent heterogeneous renumbeering

909f575

remove thrust calls from sampling post processing tests

eafe349

convert post processing test file from .cu to .cpp

4798df5

add test utility functions to generate edge IDs

93e5dc4

bug fixes

e473bf8

add heterogeneous post processing validation functions

711c99a

tweak sampling post processing tests

48cc770

add a utility function to set edge type based on src & dst vertices

06f5598

add more expensive checks

47ef442

update sampling post processing validation functions

1aa9c06

update vertex renumber map validation code

5e53f65

add tests for heterogeneous post processing

10a209c

tests/sampling/sampling_heterogeneous_post_processing_test.cpp

8578fa7

edge ID renumber map validation

58a9ab3

update documentation

a73aa52

Merge branch 'branch-24.10' of https://github.com/rapidsai/cugraph in…

e407219

…to fea_heterogeneous_renumbering_impl

seunghwak requested review from a team as code owners August 8, 2024 00:38

github-actions bot added cuGraph CMake labels Aug 8, 2024

seunghwak added feature request New feature or request non-breaking Non-breaking change and removed cuGraph CMake labels Aug 8, 2024

seunghwak added this to the 24.10 milestone Aug 8, 2024

clang-format

a64859b

github-actions bot added the cuGraph label Aug 8, 2024

github-actions bot added the CMake label Aug 8, 2024

seunghwak added breaking Breaking change and removed non-breaking Non-breaking change labels Aug 8, 2024

seunghwak requested review from ChuckHastings, jnke2016 and alexbarghi-nv August 8, 2024 01:03

seunghwak added 2 commits August 7, 2024 18:48

set do_expensive_check to false

c328b8a

Merge branch 'branch-24.10' of https://github.com/rapidsai/cugraph in…

861014d

…to fea_heterogeneous_renumbering_impl

ChuckHastings approved these changes Aug 13, 2024

View reviewed changes

seunghwak added 2 commits August 15, 2024 09:20

Merge branch 'branch-24.10' of https://github.com/rapidsai/cugraph in…

871ab91

…to fea_heterogeneous_renumbering_impl

fix merge conflicts

d6770c4

alexbarghi-nv reviewed Aug 22, 2024

View reviewed changes

alexbarghi-nv approved these changes Aug 22, 2024

View reviewed changes

ChuckHastings reviewed Aug 22, 2024

View reviewed changes

seunghwak added 2 commits August 22, 2024 12:29

fix copyright year

5f3fdc4

fix a typo in documentation

caac26d

seunghwak added 7 commits August 22, 2024 12:40

remove unnecessary checks

0d21965

Merge branch 'branch-24.10' of https://github.com/rapidsai/cugraph in…

6c240d9

…to fea_heterogeneous_renumbering_impl

Merge branch 'branch-24.10' of https://github.com/rapidsai/cugraph in…

fd25d3b

…to fea_heterogeneous_renumbering_impl

Merge branch 'branch-24.10' of https://github.com/rapidsai/cugraph in…

d52c64c

…to fea_heterogeneous_renumbering_impl

Merge branch 'branch-24.10' of https://github.com/rapidsai/cugraph in…

3c87c43

…to fea_heterogeneous_renumbering_impl

Merge branch 'branch-24.10' of https://github.com/rapidsai/cugraph in…

a921b99

…to fea_heterogeneous_renumbering_impl

remove a possibly erroneous check

01eeb1d

rapids-bot bot merged commit 338e5e0 into rapidsai:branch-24.10 Sep 3, 2024
104 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Heterogeneous renumbering implementation #4602

Heterogeneous renumbering implementation #4602

seunghwak commented Aug 8, 2024 •

edited by ChuckHastings

Loading

ChuckHastings left a comment

ChuckHastings Aug 13, 2024

seunghwak Aug 21, 2024

ChuckHastings Aug 22, 2024

seunghwak Aug 22, 2024

ChuckHastings Aug 13, 2024

alexbarghi-nv Aug 22, 2024

alexbarghi-nv left a comment

ChuckHastings Aug 22, 2024

seunghwak Aug 22, 2024

ChuckHastings commented Aug 22, 2024

alexbarghi-nv commented Aug 22, 2024

alexbarghi-nv commented Aug 22, 2024

seunghwak commented Aug 22, 2024

ChuckHastings commented Sep 3, 2024

		@@ -0,0 +1,828 @@
		/*
		* Copyright (c) 2022-2024, NVIDIA CORPORATION.

	* is only one edge type), there shouldb be only one edge source vertex type and only one edge
	* is only one edge type), there should be only one edge source vertex type and only one edge

Heterogeneous renumbering implementation #4602

Heterogeneous renumbering implementation #4602

Conversation

seunghwak commented Aug 8, 2024 • edited by ChuckHastings Loading

ChuckHastings left a 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

alexbarghi-nv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChuckHastings commented Aug 22, 2024

alexbarghi-nv commented Aug 22, 2024

alexbarghi-nv commented Aug 22, 2024

seunghwak commented Aug 22, 2024

ChuckHastings commented Sep 3, 2024

seunghwak commented Aug 8, 2024 •

edited by ChuckHastings

Loading