Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the python function symmetrizing the edgelist #4649

Merged
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
f2f3a04
add new symmetrize file
jnke2016 Sep 10, 2024
0c8b927
implement symmetrize_edgelist in the CAPI
jnke2016 Sep 10, 2024
c01456a
deprecated the python symmetrize function
jnke2016 Sep 19, 2024
fb115da
expose flag symmetrizing the edgelist
jnke2016 Sep 19, 2024
47b0677
Remove python symmetrize from the SG graph creation
jnke2016 Sep 22, 2024
f09fd41
update API to symmetrize the edgelist and update docstring
jnke2016 Sep 23, 2024
b737b02
deprecate python method to symmetrize the edgelist
jnke2016 Sep 23, 2024
ea19b62
fix style
jnke2016 Sep 23, 2024
bd7eede
fix style
jnke2016 Sep 23, 2024
6e433ab
Merge remote-tracking branch 'upstream/branch-24.10' into branch-24.1…
jnke2016 Sep 23, 2024
e5c94f7
fix typo
jnke2016 Sep 23, 2024
0470299
update docstrings and remove unused function declaration
jnke2016 Sep 26, 2024
2df98f5
ensure the graph properties match when symmetrizing
jnke2016 Sep 26, 2024
fabcb37
update docstrings
jnke2016 Sep 26, 2024
3b1c1d0
update docstrings
jnke2016 Sep 26, 2024
0462529
update error reporting
jnke2016 Sep 28, 2024
e8beb72
support the symmetrization of edgelist for legacy algorithms
jnke2016 Sep 28, 2024
e3e413b
fix typo
jnke2016 Sep 29, 2024
5336a91
fix property_graph tests
jnke2016 Sep 29, 2024
3b40fc5
update graph creation for the C tests
jnke2016 Sep 29, 2024
cbbfb88
update the test to call the deprecated symmetrize
jnke2016 Oct 1, 2024
8891b1f
fix typo
jnke2016 Oct 1, 2024
4e9e85d
fix typo
jnke2016 Oct 1, 2024
d135ac1
undo changes to symmetrize
jnke2016 Oct 1, 2024
a9c7064
fix typo
jnke2016 Oct 1, 2024
1c70046
use the deprecated symmetrize call
jnke2016 Oct 1, 2024
2cfb0fb
update SG graph creation
jnke2016 Oct 1, 2024
fec091f
Merge remote-tracking branch 'upstream/branch-24.10' into branch-24.1…
jnke2016 Oct 1, 2024
efbdb38
fix style
jnke2016 Oct 1, 2024
35c25db
fix style
jnke2016 Oct 1, 2024
f91bb81
remove debug print and pass kwarg name
jnke2016 Oct 2, 2024
0389f27
call deprecated symmetrize call
jnke2016 Oct 2, 2024
97472cd
update flag passed to the deprecated symmetrize function
jnke2016 Oct 2, 2024
ec7da8d
fix style
jnke2016 Oct 2, 2024
0e5a9ba
update docstrings
jnke2016 Oct 2, 2024
2a78356
remove redundant assignments
jnke2016 Oct 2, 2024
43ba0cd
ignore deprecation warning for symmetrizing
jnke2016 Oct 2, 2024
cfecd90
symmetrize
jnke2016 Oct 2, 2024
96d2e20
fix style
jnke2016 Oct 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion cpp/include/cugraph_c/graph.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2023, NVIDIA CORPORATION.
* Copyright (c) 2021-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -105,6 +105,8 @@ cugraph_error_code_t cugraph_sg_graph_create(
weights,
* or take the maximum weight), the caller should remove specific edges themselves and not rely
* on this flag.
* @param [in] symmetrize If true, symmetrize the edgelist. The symmetrization of edges
* with edge_ids and/or edge_type_ids is currently not supported.
* @param [in] do_expensive_check If true, do expensive checks to validate the input data
* is consistent with software assumptions. If false bypass these checks.
* @param [out] graph A pointer to the graph object
Expand All @@ -126,6 +128,7 @@ cugraph_error_code_t cugraph_graph_create_sg(
bool_t renumber,
bool_t drop_self_loops,
bool_t drop_multi_edges,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_graph_t** graph,
cugraph_error_t** error);
Expand All @@ -150,6 +153,8 @@ cugraph_error_code_t cugraph_graph_create_sg(
* If false, do not renumber. Renumbering enables some significant optimizations within
* the graph primitives library, so it is strongly encouraged. Renumbering is required if
* the vertices are not sequential integer values from 0 to num_vertices.
* @param [in] symmetrize If true, symmetrize the edgelist. The symmetrization of edges
* with edge_ids and/or edge_type_ids is currently not supported.
* @param [in] do_expensive_check If true, do expensive checks to validate the input data
* is consistent with software assumptions. If false bypass these checks.
* @param [out] graph A pointer to the graph object
Expand All @@ -168,6 +173,7 @@ cugraph_error_code_t cugraph_sg_graph_create_from_csr(
const cugraph_type_erased_device_array_view_t* edge_type_ids,
bool_t store_transposed,
bool_t renumber,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_graph_t** graph,
cugraph_error_t** error);
Expand All @@ -190,6 +196,8 @@ cugraph_error_code_t cugraph_sg_graph_create_from_csr(
* If false, do not renumber. Renumbering enables some significant optimizations within
* the graph primitives library, so it is strongly encouraged. Renumbering is required if
* the vertices are not sequential integer values from 0 to num_vertices.
* @param [in] symmetrize If true, symmetrize the edgelist. The symmetrization of edges
* with edge_ids and/or edge_type_ids is currently not supported.
* @param [in] do_expensive_check If true, do expensive checks to validate the input data
* is consistent with software assumptions. If false bypass these checks.
* @param [out] graph A pointer to the graph object
Expand All @@ -208,6 +216,7 @@ cugraph_error_code_t cugraph_graph_create_sg_from_csr(
const cugraph_type_erased_device_array_view_t* edge_type_ids,
bool_t store_transposed,
bool_t renumber,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_graph_t** graph,
cugraph_error_t** error);
Expand Down Expand Up @@ -289,6 +298,8 @@ cugraph_error_code_t cugraph_mg_graph_create(
* Note that setting this flag will arbitrarily select one instance of a multi edge to be the
* edge that survives. If the edges have properties that should be honored (e.g. sum the
* weights, or take the maximum weight), the caller should do that on not rely on this flag.
* @param [in] symmetrize If true, symmetrize the edgelist. The symmetrization of edges
* with edge_ids and/or edge_type_ids is currently not supported.
* @param [in] do_expensive_check If true, do expensive checks to validate the input data
* is consistent with software assumptions. If false bypass these checks.
* @param [out] graph A pointer to the graph object
Expand All @@ -309,6 +320,7 @@ cugraph_error_code_t cugraph_graph_create_mg(
size_t num_arrays,
bool_t drop_self_loops,
bool_t drop_multi_edges,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_graph_t** graph,
cugraph_error_t** error);
Expand Down
30 changes: 30 additions & 0 deletions cpp/src/c_api/graph_mg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ struct create_graph_functor : public cugraph::c_api::abstract_functor {
bool_t renumber_;
bool_t drop_self_loops_;
bool_t drop_multi_edges_;
bool_t symmetrize_;
bool_t do_expensive_check_;
cugraph::c_api::cugraph_graph_t* result_{};

Expand All @@ -91,6 +92,7 @@ struct create_graph_functor : public cugraph::c_api::abstract_functor {
bool_t renumber,
bool_t drop_self_loops,
bool_t drop_multi_edges,
bool_t symmetrize,
bool_t do_expensive_check)
: abstract_functor(),
properties_(properties),
Expand All @@ -109,6 +111,7 @@ struct create_graph_functor : public cugraph::c_api::abstract_functor {
renumber_(renumber),
drop_self_loops_(drop_self_loops),
drop_multi_edges_(drop_multi_edges),
symmetrize_(symmetrize),
do_expensive_check_(do_expensive_check)
{
}
Expand Down Expand Up @@ -224,6 +227,22 @@ struct create_graph_functor : public cugraph::c_api::abstract_functor {
: false);
}

if (symmetrize_) {
if (edgelist_edge_ids || edgelist_edge_types) {
// Currently doesn't support the symmetrization of edgelist with edge_ids and edge_types
unsupported();
}

// Symmetrize the edgelist
std::tie(edgelist_srcs, edgelist_dsts, edgelist_weights) =
cugraph::symmetrize_edgelist<vertex_t, weight_t, store_transposed, multi_gpu>(
handle_,
std::move(edgelist_srcs),
std::move(edgelist_dsts),
std::move(edgelist_weights),
false);
}

std::tie(*graph, new_edge_weights, new_edge_ids, new_edge_types, new_number_map) =
cugraph::create_graph_from_edgelist<vertex_t,
edge_t,
Expand Down Expand Up @@ -290,6 +309,7 @@ extern "C" cugraph_error_code_t cugraph_graph_create_mg(
size_t num_arrays,
bool_t drop_self_loops,
bool_t drop_multi_edges,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_graph_t** graph,
cugraph_error_t** error)
Expand Down Expand Up @@ -358,6 +378,14 @@ extern "C" cugraph_error_code_t cugraph_graph_create_mg(
if (weight_type == cugraph_data_type_id_t::NTYPES) weight_type = p_weights[i]->type_;
}

if (symmetrize == TRUE) {
CAPI_EXPECTS((properties->is_symmetric == TRUE),
CUGRAPH_INVALID_INPUT,
"Invalid input arguments: The graph property must be symmetric if 'symmetrize' "
"is set to True.",
*error);
}

CAPI_EXPECTS(p_src[i]->type_ == vertex_type,
CUGRAPH_INVALID_INPUT,
"Invalid input arguments: all vertex types must match",
Expand Down Expand Up @@ -488,6 +516,7 @@ extern "C" cugraph_error_code_t cugraph_graph_create_mg(
bool_t::TRUE,
drop_self_loops,
drop_multi_edges,
symmetrize,
do_expensive_check);

try {
Expand Down Expand Up @@ -534,6 +563,7 @@ extern "C" cugraph_error_code_t cugraph_mg_graph_create(
1,
FALSE,
FALSE,
FALSE,
do_expensive_check,
graph,
error);
Expand Down
61 changes: 61 additions & 0 deletions cpp/src/c_api/graph_sg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ struct create_graph_functor : public cugraph::c_api::abstract_functor {
bool_t renumber_;
bool_t drop_self_loops_;
bool_t drop_multi_edges_;
bool_t symmetrize_;
bool_t do_expensive_check_;
cugraph_data_type_id_t edge_type_;
cugraph::c_api::cugraph_graph_t* result_{};
Expand All @@ -58,6 +59,7 @@ struct create_graph_functor : public cugraph::c_api::abstract_functor {
bool_t renumber,
bool_t drop_self_loops,
bool_t drop_multi_edges,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_data_type_id_t edge_type)
: abstract_functor(),
Expand All @@ -72,6 +74,7 @@ struct create_graph_functor : public cugraph::c_api::abstract_functor {
renumber_(renumber),
drop_self_loops_(drop_self_loops),
drop_multi_edges_(drop_multi_edges),
symmetrize_(symmetrize),
do_expensive_check_(do_expensive_check),
edge_type_(edge_type)
{
Expand Down Expand Up @@ -207,6 +210,22 @@ struct create_graph_functor : public cugraph::c_api::abstract_functor {
: false);
}

if (symmetrize_) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to this logic, if symmetrize_ is true then we should override the graph properties to set is_symmetric to true.

if (edgelist_edge_ids || edgelist_edge_types) {
// Currently doesn't support the symmetrization with edge_ids and edge_types
unsupported();
}

// Symmetrize the edgelist
std::tie(edgelist_srcs, edgelist_dsts, edgelist_weights) =
cugraph::symmetrize_edgelist<vertex_t, weight_t, store_transposed, multi_gpu>(
handle_,
std::move(edgelist_srcs),
std::move(edgelist_dsts),
std::move(edgelist_weights),
false);
}

std::tie(*graph, new_edge_weights, new_edge_ids, new_edge_types, new_number_map) =
cugraph::create_graph_from_edgelist<vertex_t,
edge_t,
Expand Down Expand Up @@ -268,6 +287,7 @@ struct create_graph_csr_functor : public cugraph::c_api::abstract_functor {
cugraph::c_api::cugraph_type_erased_device_array_view_t const* edge_ids_;
cugraph::c_api::cugraph_type_erased_device_array_view_t const* edge_type_ids_;
bool_t renumber_;
bool_t symmetrize_;
bool_t do_expensive_check_;
cugraph::c_api::cugraph_graph_t* result_{};

Expand All @@ -280,6 +300,7 @@ struct create_graph_csr_functor : public cugraph::c_api::abstract_functor {
cugraph::c_api::cugraph_type_erased_device_array_view_t const* edge_ids,
cugraph::c_api::cugraph_type_erased_device_array_view_t const* edge_type_ids,
bool_t renumber,
bool_t symmetrize,
bool_t do_expensive_check)
: abstract_functor(),
properties_(properties),
Expand All @@ -290,6 +311,7 @@ struct create_graph_csr_functor : public cugraph::c_api::abstract_functor {
edge_ids_(edge_ids),
edge_type_ids_(edge_type_ids),
renumber_(renumber),
symmetrize_(symmetrize),
do_expensive_check_(do_expensive_check)
{
}
Expand Down Expand Up @@ -398,6 +420,22 @@ struct create_graph_csr_functor : public cugraph::c_api::abstract_functor {
cugraph::graph_view_t<vertex_t, edge_t, store_transposed, multi_gpu>,
edge_type_id_t>(handle_);

if (symmetrize_) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above regarding is_symmetric

if (edgelist_edge_ids || edgelist_edge_types) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add an issue to discuss what symmetrization means with respect to edge properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tracked this in an issue

// Currently doesn't support the symmetrization with edge_ids and edge_types
unsupported();
}

// Symmetrize the edgelist
std::tie(edgelist_srcs, edgelist_dsts, edgelist_weights) =
cugraph::symmetrize_edgelist<vertex_t, weight_t, store_transposed, multi_gpu>(
handle_,
std::move(edgelist_srcs),
std::move(edgelist_dsts),
std::move(edgelist_weights),
false);
}

std::tie(*graph, new_edge_weights, new_edge_ids, new_edge_types, new_number_map) =
cugraph::create_graph_from_edgelist<vertex_t,
edge_t,
Expand Down Expand Up @@ -518,6 +556,7 @@ extern "C" cugraph_error_code_t cugraph_graph_create_sg(
bool_t renumber,
bool_t drop_self_loops,
bool_t drop_multi_edges,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_graph_t** graph,
cugraph_error_t** error)
Expand All @@ -542,6 +581,14 @@ extern "C" cugraph_error_code_t cugraph_graph_create_sg(
auto p_edge_type_ids =
reinterpret_cast<cugraph::c_api::cugraph_type_erased_device_array_view_t const*>(edge_type_ids);

if (symmetrize == TRUE) {
CAPI_EXPECTS((properties->is_symmetric == TRUE),
CUGRAPH_INVALID_INPUT,
"Invalid input arguments: The graph property must be symmetric if 'symmetrize' is "
"set to True.",
*error);
}

CAPI_EXPECTS(p_src->size_ == p_dst->size_,
CUGRAPH_INVALID_INPUT,
"Invalid input arguments: src size != dst size.",
Expand Down Expand Up @@ -606,6 +653,7 @@ extern "C" cugraph_error_code_t cugraph_graph_create_sg(
renumber,
drop_self_loops,
drop_multi_edges,
symmetrize,
do_expensive_check,
edge_type);

Expand Down Expand Up @@ -658,6 +706,7 @@ extern "C" cugraph_error_code_t cugraph_sg_graph_create(
renumber,
FALSE,
FALSE,
FALSE,
do_expensive_check,
graph,
error);
Expand All @@ -673,6 +722,7 @@ cugraph_error_code_t cugraph_graph_create_sg_from_csr(
const cugraph_type_erased_device_array_view_t* edge_type_ids,
bool_t store_transposed,
bool_t renumber,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_graph_t** graph,
cugraph_error_t** error)
Expand Down Expand Up @@ -707,6 +757,14 @@ cugraph_error_code_t cugraph_graph_create_sg_from_csr(
weight_type = cugraph_data_type_id_t::FLOAT32;
}

if (symmetrize == TRUE) {
CAPI_EXPECTS((properties->is_symmetric == TRUE),
CUGRAPH_INVALID_INPUT,
"Invalid input arguments: The graph property must be symmetric if 'symmetrize' is "
"set to True.",
*error);
}

CAPI_EXPECTS(
(edge_type_ids == nullptr && edge_ids == nullptr) ||
(edge_type_ids != nullptr && edge_ids != nullptr),
Expand Down Expand Up @@ -735,6 +793,7 @@ cugraph_error_code_t cugraph_graph_create_sg_from_csr(
p_edge_ids,
p_edge_type_ids,
renumber,
FALSE, // symmetrize
do_expensive_check);

try {
Expand Down Expand Up @@ -770,6 +829,7 @@ cugraph_error_code_t cugraph_sg_graph_create_from_csr(
const cugraph_type_erased_device_array_view_t* edge_type_ids,
bool_t store_transposed,
bool_t renumber,
bool_t symmetrize,
bool_t do_expensive_check,
cugraph_graph_t** graph,
cugraph_error_t** error)
Expand All @@ -783,6 +843,7 @@ cugraph_error_code_t cugraph_sg_graph_create_from_csr(
edge_type_ids,
store_transposed,
renumber,
symmetrize,
do_expensive_check,
graph,
error);
Expand Down
6 changes: 6 additions & 0 deletions cpp/tests/c_api/create_graph_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ int test_create_sg_graph_simple()
FALSE,
FALSE,
FALSE,
FALSE,
&graph,
&ret_error);
TEST_ASSERT(test_ret_value, ret_code == CUGRAPH_SUCCESS, "graph creation failed.");
Expand Down Expand Up @@ -213,6 +214,7 @@ int test_create_sg_graph_csr()
FALSE,
FALSE,
FALSE,
FALSE,
&graph,
&ret_error);
TEST_ASSERT(test_ret_value, ret_code == CUGRAPH_SUCCESS, "graph creation failed.");
Expand Down Expand Up @@ -408,6 +410,7 @@ int test_create_sg_graph_symmetric_error()
FALSE,
FALSE,
FALSE,
FALSE,
TRUE,
&graph,
&ret_error);
Expand Down Expand Up @@ -526,6 +529,7 @@ int test_create_sg_graph_with_isolated_vertices()
FALSE,
FALSE,
FALSE,
FALSE,
&graph,
&ret_error);
TEST_ASSERT(test_ret_value, ret_code == CUGRAPH_SUCCESS, "graph creation failed.");
Expand Down Expand Up @@ -675,6 +679,7 @@ int test_create_sg_graph_csr_with_isolated()
FALSE,
FALSE,
FALSE,
FALSE,
&graph,
&ret_error);
TEST_ASSERT(test_ret_value, ret_code == CUGRAPH_SUCCESS, "graph creation failed.");
Expand Down Expand Up @@ -840,6 +845,7 @@ int test_create_sg_graph_with_isolated_vertices_multi_input()
TRUE,
TRUE,
FALSE,
FALSE,
&graph,
&ret_error);
TEST_ASSERT(test_ret_value, ret_code == CUGRAPH_SUCCESS, "graph creation failed.");
Expand Down
Loading
Loading