From 32ddd768e8cd8031a3377236125379d93b972e53 Mon Sep 17 00:00:00 2001 From: Charles Hastings Date: Tue, 8 Nov 2022 16:10:17 -0800 Subject: [PATCH 1/3] fix setting of graph properties during transpose_storage function --- cpp/src/c_api/core_number.cpp | 2 -- .../transpose_graph_storage_impl.cuh | 15 +++++++++++++-- .../c_api/weakly_connected_components_test.c | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/cpp/src/c_api/core_number.cpp b/cpp/src/c_api/core_number.cpp index 722f657fc24..d4bc76ceaeb 100644 --- a/cpp/src/c_api/core_number.cpp +++ b/cpp/src/c_api/core_number.cpp @@ -67,8 +67,6 @@ struct core_number_functor : public cugraph::c_api::abstract_functor { if (error_code_ != CUGRAPH_SUCCESS) ; } - // FIXME: Transpose_storage may have a bug, since if store_transposed is True it can reverse - // the bool value of is_symmetric auto graph = reinterpret_cast*>( graph_->graph_); diff --git a/cpp/src/structure/transpose_graph_storage_impl.cuh b/cpp/src/structure/transpose_graph_storage_impl.cuh index 193705c876a..bbc21dd998c 100644 --- a/cpp/src/structure/transpose_graph_storage_impl.cuh +++ b/cpp/src/structure/transpose_graph_storage_impl.cuh @@ -62,6 +62,10 @@ transpose_graph_storage_impl( } auto is_multigraph = graph.is_multigraph(); + auto is_symmetric = graph.is_symmetric(); + + // FIXME: if is_symmetric is true we can do this more efficiently, + // since the graph contents should be exactly the same auto [edgelist_srcs, edgelist_dsts, edgelist_weights] = decompress_to_edgelist(handle, @@ -89,7 +93,7 @@ transpose_graph_storage_impl( std::move(edgelist_dsts), std::move(edgelist_weights), std::nullopt, - graph_properties_t{is_multigraph, false}, + graph_properties_t{is_symmetric, is_multigraph}, true); return std::make_tuple(std::move(storage_transposed_graph), std::move(new_renumber_map)); @@ -123,6 +127,10 @@ transpose_graph_storage_impl( auto number_of_vertices = graph.number_of_vertices(); auto is_multigraph = graph.is_multigraph(); bool renumber = renumber_map.has_value(); + auto is_symmetric = graph.is_symmetric(); + + // FIXME: if is_symmetric is true we can do this more efficiently, + // since the graph contents should be exactly the same auto [edgelist_srcs, edgelist_dsts, edgelist_weights] = decompress_to_edgelist(handle, @@ -150,9 +158,12 @@ transpose_graph_storage_impl( std::move(edgelist_dsts), std::move(edgelist_weights), std::nullopt, - graph_properties_t{is_multigraph, false}, + graph_properties_t{is_symmetric, is_multigraph}, renumber); + std::cout << "should have created graph with is_symmetric = " << (is_symmetric ? "TRUE" : "FALSE") << std::endl; + std::cout << " store_transposed_graph is_symmetric = " << (storage_transposed_graph.is_symmetric() ? "TRUE" : "FALSE") << std::endl; + return std::make_tuple(std::move(storage_transposed_graph), std::move(new_renumber_map)); } diff --git a/cpp/tests/c_api/weakly_connected_components_test.c b/cpp/tests/c_api/weakly_connected_components_test.c index 4f711b4fcde..4571837bc0b 100644 --- a/cpp/tests/c_api/weakly_connected_components_test.c +++ b/cpp/tests/c_api/weakly_connected_components_test.c @@ -117,11 +117,30 @@ int test_weakly_connected_components() return generic_wcc_test(h_src, h_dst, h_wgt, h_result, num_vertices, num_edges, FALSE); } +int test_weakly_connected_components_transpose() +{ + size_t num_edges = 32; + size_t num_vertices = 12; + + vertex_t h_src[] = {0, 1, 1, 2, 2, 2, 3, 4, 6, 7, 7, 8, 8, 8, 9, 10, + 1, 3, 4, 0, 1, 3, 5, 5, 7, 9, 10, 6, 7, 9, 11, 11}; + vertex_t h_dst[] = {1, 3, 4, 0, 1, 3, 5, 5, 7, 9, 10, 6, 7, 9, 11, 11, + 0, 1, 1, 2, 2, 2, 3, 4, 6, 7, 7, 8, 8, 8, 9, 10}; + weight_t h_wgt[] = {1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, + 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, + 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0}; + vertex_t h_result[] = {0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1}; + + // WCC wants store_transposed = FALSE + return generic_wcc_test(h_src, h_dst, h_wgt, h_result, num_vertices, num_edges, TRUE); +} + /******************************************************************************/ int main(int argc, char** argv) { int result = 0; result |= RUN_TEST(test_weakly_connected_components); + result |= RUN_TEST(test_weakly_connected_components_transpose); return result; } From e5389df0e1eeb812999e4d6ab17d0218192d6592 Mon Sep 17 00:00:00 2001 From: Charles Hastings Date: Tue, 8 Nov 2022 19:23:33 -0800 Subject: [PATCH 2/3] fix clang-format issues --- cpp/src/structure/transpose_graph_storage_impl.cuh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cpp/src/structure/transpose_graph_storage_impl.cuh b/cpp/src/structure/transpose_graph_storage_impl.cuh index bbc21dd998c..feefda08635 100644 --- a/cpp/src/structure/transpose_graph_storage_impl.cuh +++ b/cpp/src/structure/transpose_graph_storage_impl.cuh @@ -62,7 +62,7 @@ transpose_graph_storage_impl( } auto is_multigraph = graph.is_multigraph(); - auto is_symmetric = graph.is_symmetric(); + auto is_symmetric = graph.is_symmetric(); // FIXME: if is_symmetric is true we can do this more efficiently, // since the graph contents should be exactly the same @@ -127,7 +127,7 @@ transpose_graph_storage_impl( auto number_of_vertices = graph.number_of_vertices(); auto is_multigraph = graph.is_multigraph(); bool renumber = renumber_map.has_value(); - auto is_symmetric = graph.is_symmetric(); + auto is_symmetric = graph.is_symmetric(); // FIXME: if is_symmetric is true we can do this more efficiently, // since the graph contents should be exactly the same @@ -161,8 +161,10 @@ transpose_graph_storage_impl( graph_properties_t{is_symmetric, is_multigraph}, renumber); - std::cout << "should have created graph with is_symmetric = " << (is_symmetric ? "TRUE" : "FALSE") << std::endl; - std::cout << " store_transposed_graph is_symmetric = " << (storage_transposed_graph.is_symmetric() ? "TRUE" : "FALSE") << std::endl; + std::cout << "should have created graph with is_symmetric = " << (is_symmetric ? "TRUE" : "FALSE") + << std::endl; + std::cout << " store_transposed_graph is_symmetric = " + << (storage_transposed_graph.is_symmetric() ? "TRUE" : "FALSE") << std::endl; return std::make_tuple(std::move(storage_transposed_graph), std::move(new_renumber_map)); } From 8f7d29f8f52aa1e2b029e18742ab59f549bb1de5 Mon Sep 17 00:00:00 2001 From: Charles Hastings Date: Wed, 9 Nov 2022 11:57:56 -0800 Subject: [PATCH 3/3] noticed I left in some print statements, also same bug exists in transpose_graph --- cpp/src/structure/transpose_graph_impl.cuh | 4 ++-- cpp/src/structure/transpose_graph_storage_impl.cuh | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/cpp/src/structure/transpose_graph_impl.cuh b/cpp/src/structure/transpose_graph_impl.cuh index 135245b062d..8a079a788c4 100644 --- a/cpp/src/structure/transpose_graph_impl.cuh +++ b/cpp/src/structure/transpose_graph_impl.cuh @@ -89,7 +89,7 @@ transpose_graph_impl(raft::handle_t const& handle, std::move(edgelist_srcs), std::move(edgelist_weights), std::nullopt, - graph_properties_t{is_multigraph, false}, + graph_properties_t{false, is_multigraph}, true); return std::make_tuple(std::move(transposed_graph), std::move(new_renumber_map)); @@ -150,7 +150,7 @@ transpose_graph_impl(raft::handle_t const& handle, std::move(edgelist_srcs), std::move(edgelist_weights), std::nullopt, - graph_properties_t{is_multigraph, false}, + graph_properties_t{false, is_multigraph}, renumber); return std::make_tuple(std::move(transposed_graph), std::move(new_renumber_map)); diff --git a/cpp/src/structure/transpose_graph_storage_impl.cuh b/cpp/src/structure/transpose_graph_storage_impl.cuh index feefda08635..7a5857452bb 100644 --- a/cpp/src/structure/transpose_graph_storage_impl.cuh +++ b/cpp/src/structure/transpose_graph_storage_impl.cuh @@ -161,11 +161,6 @@ transpose_graph_storage_impl( graph_properties_t{is_symmetric, is_multigraph}, renumber); - std::cout << "should have created graph with is_symmetric = " << (is_symmetric ? "TRUE" : "FALSE") - << std::endl; - std::cout << " store_transposed_graph is_symmetric = " - << (storage_transposed_graph.is_symmetric() ? "TRUE" : "FALSE") << std::endl; - return std::make_tuple(std::move(storage_transposed_graph), std::move(new_renumber_map)); }