Skip to content

Commit

Permalink
Fix bug in how is_symmetric is set when transposing storage (#2898)
Browse files Browse the repository at this point in the history
The `tranpose_storage` function was incorrectly setting graph properties.

Authors:
  - Chuck Hastings (https://github.com/ChuckHastings)

Approvers:
  - Joseph Nke (https://github.com/jnke2016)
  - Seunghwa Kang (https://github.com/seunghwak)

URL: #2898
  • Loading branch information
ChuckHastings authored Nov 9, 2022
1 parent 7387fbc commit fd0a2be
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 6 deletions.
2 changes: 0 additions & 2 deletions cpp/src/c_api/core_number.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<cugraph::graph_t<vertex_t, edge_t, weight_t, false, multi_gpu>*>(
graph_->graph_);
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/structure/transpose_graph_impl.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
12 changes: 10 additions & 2 deletions cpp/src/structure/transpose_graph_storage_impl.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -150,7 +158,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},
renumber);

return std::make_tuple(std::move(storage_transposed_graph), std::move(new_renumber_map));
Expand Down
19 changes: 19 additions & 0 deletions cpp/tests/c_api/weakly_connected_components_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit fd0a2be

Please sign in to comment.