Move edge triangle count to the stable API by jnke2016 · Pull Request #4382 · rapidsai/cugraph

jnke2016 · 2024-04-30T15:40:59Z

This PR

Performs edge triangle count in chunk
Enables k - 1 core optimization
Add C++ tests for edge triangle count
Move edge triangle count to the stable API
Implement MG edge triangle count and add tests
Update 'mg_graph_to_sg_graph' to support 'edge_ids' along with tests

closes #4370
closes #4371

…6_stable-edge_triangle-count

… into branch-24.06_stable-edge_triangle-count

seunghwak

I haven't reviewed the actual implementation, just a quick first review.

cpp/include/cugraph/algorithms.hpp

seunghwak · 2024-05-06T17:13:01Z

cpp/tests/community/edge_triangle_count_test.cpp

+#include <vector>
+
+struct EdgeTriangleCount_Usecase {
+  bool test_weighted_{false};


Need to include edge_masking.

And we need mg_edge_triangle_count_test as well (is this code MNMG ready?)

Need to include edge_masking.

I included edge mask tests

is this code MNMG ready?

Yes it is

cpp/include/cugraph/algorithms.hpp

seunghwak

Reviewed the edge triangle count implementation (Haven't reviewed the K-Truss implementation updates yet).

cpp/src/community/edge_triangle_count_impl.cuh

seunghwak · 2024-05-06T17:32:30Z

cpp/src/community/edge_triangle_count_impl.cuh

-                             std::array<bool, 2>{true, true},
-                             false /*FIXME: pass 'do_expensive_check' as argument*/);
+  size_t approx_edges_to_intersect_per_iteration =
+    static_cast<size_t>(handle.get_device_properties().multiProcessorCount) * (1 << 17);


This isn't approximate, right? (I mean except for the last chunk). This approx_ naming convention is used when we are dealing with the CSR data structure and # edges should need to be aligned with the CSR offset array boundaries.

I mean except for the last chunk

Ya or if it is bigger than the number of edges. But I agree it doesn't align with the intended context (CSR data structure). I renamed it to edges_to_intersect_per_iteration

seunghwak · 2024-05-06T17:32:52Z

cpp/src/community/edge_triangle_count_impl.cuh

+
  auto edge_first = thrust::make_zip_iterator(edgelist_srcs.begin(), edgelist_dsts.begin());

  thrust::sort(handle.get_thrust_policy(), edge_first, edge_first + edgelist_srcs.size());


What's the purpose of sort here?

Good catch. We no longer need this as we do not pass an edgelist to edge_triangle_count. In fact when we decompress the edges from the graph, they are already sorted.

cpp/src/community/edge_triangle_count_impl.cuh

seunghwak · 2024-05-06T17:46:11Z

cpp/src/community/edge_triangle_count_impl.cuh

+                             ? (edgelist_srcs.size() / approx_edges_to_intersect_per_iteration)
+                             : (edgelist_srcs.size() / approx_edges_to_intersect_per_iteration) + 1;
+  size_t prev_chunk_size = 0;
+  auto num_edges         = edgelist_srcs.size();


num_remaining_edges to be clearer?

And prev_chunk_size = edgelist_srcs.size() - num_remaining_edges, so redundant.

num_remaining_edges to be clearer?

Done

And prev_chunk_size = edgelist_srcs.size() - num_remaining_edges, so redundant

Can you be more clear here?

seunghwak

OK, finished the first round review.

seunghwak · 2024-05-06T18:02:57Z

cpp/src/community/k_truss_impl.cuh

        std::nullopt,
        cugraph::graph_properties_t{true, graph_view.is_multigraph()},
-        true);
+        false);  // FIXME: Renumbering should not be hardcoded.


What is this FIXME?

I wasn't sure if renumbering should be hardcoded but I removed the FIXME.

cpp/src/community/k_truss_impl.cuh

… into branch-24.06_stable-edge_triangle-count

…6_optimize-k-truss

… into branch-24.06_stable-edge_triangle-count

ChuckHastings · 2024-05-15T22:30:55Z

cpp/src/community/edge_triangle_count_impl.cuh

+          std::move(std::get<0>(vertex_pair_buffer)),
+          std::move(std::get<1>(vertex_pair_buffer)),
+          std::nullopt,
+          // FIXME: Update 'shuffle_int_...' to support int32_t and int64_t values


Can you clarify this FIXME? You can use int32_t and int64_t values as you are currently doing this.

You are calling a function that was designed to shuffle vertex pairs along with edge weight, edge id and edge type around the GPU cluster. That's not what you have, you have vertex pairs and an increase count. If this function didn't exist, since you are in a .cu file you could replace this logic with something akin to this:

cugraph/cpp/src/utilities/shuffle_vertex_pairs.cu

Line 52 in a77840e

auto& comm = handle.get_comms();

If your concern is that we should have a more general purpose function for shuffling vertex pairs and arbitrary attributes then be sure to specify what you want.

You can use int32_t and int64_t values as you are currently doing this.

Right. This is currently how we proceed but based on Seunghwa feedbacks on decompress_edgelist, we should have something separate from edge_id to handle this in case edge_id and count don't hold the same type. I thought the same holds for the shuffling function.

I guess I'm asking for a more descriptive FIXME. As worded it is not clear to me what you are asking to be changed at some point in the future.

Right. I updated the FIXME and it will be available in the next commit

ChuckHastings · 2024-05-15T23:18:08Z

cpp/tests/utilities/conversion_utilities.hpp

  raft::handle_t const& handle,
  cugraph::graph_view_t<vertex_t, edge_t, store_transposed, true> const& graph_view,
  std::optional<cugraph::edge_property_view_t<edge_t, weight_t const*>> edge_weight_view,
+  std::optional<cugraph::edge_property_view_t<edge_t, edge_t const*>> edge_id_view,


Should we also add edge_type here, since we're modifying all of the functions? We do currently support edge weight, edge id and edge type in most of our functions.

… into branch-24.06_stable-edge_triangle-count

…6_stable-edge_triangle-count

seunghwak

LGTM

…6_stable-edge_triangle-count

…gle-count' into branch-24.06_stable-edge_triangle-count

BradReesWork · 2024-05-28T13:40:22Z

/merge

jnke2016 added 4 commits April 25, 2024 08:16

enable k-1 core

be543b1

perform edge triangle count in chunk

327a07e

perform edge triangle count in chunks

c623465

enable k-1 core optimization and unroll intersection edges in chunks

7cbb61f

jnke2016 requested a review from a team as a code owner April 30, 2024 15:41

github-actions bot added the cuGraph label Apr 30, 2024

jnke2016 added 2 commits April 30, 2024 08:42

fix style

6dbe157

fix style

3e12f02

alexbarghi-nv added improvement Improvement / enhancement to an existing function breaking Breaking change and removed cuGraph labels May 1, 2024

jnke2016 added 5 commits May 4, 2024 03:40

add edge triangle count tests

1f00dd6

move edge triangle count to the stable API

da330b3

fix style

913283d

update branch

d758071

fix style

ce3ff47

jnke2016 requested a review from a team as a code owner May 4, 2024 11:06

github-actions bot added cuGraph CMake labels May 4, 2024

jnke2016 added 5 commits May 4, 2024 04:07

Merge remote-tracking branch 'upstream/branch-24.06' into branch-24.0…

0ebb7f5

…6_stable-edge_triangle-count

update function definition

9d2d4f7

fix style

2019d99

update test

701e33d

Merge remote-tracking branch 'upstream/branch-24.06_optimize-k-truss'…

9a12994

… into branch-24.06_stable-edge_triangle-count

seunghwak reviewed May 6, 2024

View reviewed changes

ChuckHastings reviewed May 6, 2024

View reviewed changes

cpp/include/cugraph/algorithms.hpp Show resolved Hide resolved

seunghwak reviewed May 6, 2024

View reviewed changes

jnke2016 added 2 commits May 7, 2024 07:10

return edge_property_t

0d74246

fix style

c103e52

jnke2016 added 13 commits May 14, 2024 08:06

remove temporary variable

0ca2953

remove weights test

56118f3

add edge mask test for rmat benchmark

3463723

update mg tests

f047a41

fix style

52bc273

update branch

02e15e4

fix style

2030ed8

Merge remote-tracking branch 'upstream/branch-24.06_optimize-k-truss'…

3385690

… into branch-24.06_stable-edge_triangle-count

update tests

1c4ee11

fix style

5b1c248

Merge remote-tracking branch 'upstream/branch-24.06_optimize-k-truss'…

b8e21c3

… into branch-24.06_stable-edge_triangle-count

Merge remote-tracking branch 'upstream/branch-24.06' into branch-24.0…

fefddc5

…6_optimize-k-truss

Merge remote-tracking branch 'upstream/branch-24.06_optimize-k-truss'…

9e9a198

… into branch-24.06_stable-edge_triangle-count

ChuckHastings reviewed May 15, 2024

View reviewed changes

jnke2016 added 5 commits May 15, 2024 18:27

update fixme

0ab7906

Merge remote-tracking branch 'upstream/branch-24.06_optimize-k-truss'…

6439274

… into branch-24.06_stable-edge_triangle-count

Merge remote-tracking branch 'upstream/branch-24.06' into branch-24.0…

a3e041e

…6_stable-edge_triangle-count

Merge remote-tracking branch 'upstream/branch-24.06' into branch-24.0…

d4217b2

…6_stable-edge_triangle-count

fix typo and function call

1c175dc

ChuckHastings approved these changes May 22, 2024

View reviewed changes

ChuckHastings mentioned this pull request May 22, 2024

[FEA] local bridges (similar to Nx) #1418

Closed

seunghwak approved these changes May 23, 2024

View reviewed changes

rlratzel and others added 5 commits May 24, 2024 00:34

Merge remote-tracking branch 'upstream/branch-24.06' into branch-24.0…

095d561

…6_stable-edge_triangle-count

Fixes style.

2408a95

fix style

03a2790

Merge remote-tracking branch 'upstream/branch-24.06' into branch-24.0…

cf39539

…6_stable-edge_triangle-count

Merge remote-tracking branch 'upstream/branch-24.06_stable-edge_trian…

a62ad7a

…gle-count' into branch-24.06_stable-edge_triangle-count

rapids-bot bot merged commit 1c3f3a8 into rapidsai:branch-24.06 May 28, 2024

jnke2016 mentioned this pull request Sep 18, 2025

Algorithm - Edge Triangle Counting #4304

Closed


		auto edge_first = thrust::make_zip_iterator(edgelist_srcs.begin(), edgelist_dsts.begin());

		thrust::sort(handle.get_thrust_policy(), edge_first, edge_first + edgelist_srcs.size());

Conversation

jnke2016 commented Apr 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seunghwak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seunghwak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seunghwak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seunghwak left a comment

Choose a reason for hiding this comment

Uh oh!

BradReesWork commented May 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jnke2016 commented Apr 30, 2024 •

edited

Loading