Skip to content
Draft
Changes from 1 commit
Commits
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
103 changes: 66 additions & 37 deletions cpp/include/cugraph/graph_functions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -423,15 +423,27 @@ decompress_to_edgelist(

/**
* @ingroup graph_functions_cpp
* @brief Symmetrize edgelist.
* @brief Rule for combining edge properties when adding reciprocal edges.
*/
enum class edge_property_combining_rule_t {
PART_OF_UNIQUENESS, /// This property is part of the uniqueness of the edge.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah... I am not sure about this... This is not really a combining rule...

And if this is selected for an edge property... how do we consider that in symmetrization.

Say edge ID is selected as "PART_OF_UNIQUENESS" property.

In (src, dst, ID) triplets, how do we symmetrize (3, 5, 1) and (5, 3, 1) with reciprocal = true/false? What about (3, 5, 1) and (5, 3, 2)?

I think we should make this cleaner and better document the expected behavior. Just reading the names, I can't properly guess how these functions will work.

SUM, /// Sum the properties of the two edges.
MAX, /// Take the maximum of the properties of the two edges.
MIN, /// Take the minimum of the properties of the two edges.
MEAN, /// Take the mean of the properties of the two edges.
ARBITRARY /// Arbitrarily choose one of the properties.
};

/**
* @ingroup graph_functions_cpp
* @brief Add reciprocal edges to the edgelist. This was formerly called symmetrize_edgelist
* with reciprocal set to true.
*
* Reciprocal edges are edges that appear in both directions. Edges that are only in one direction
* will be added to the edge list in the opposite direction. The resulting edge list will be
* symmetric.
*
* @tparam vertex_t Type of vertex identifiers. Needs to be an integral type.
* @tparam edge_t Type of edge identifiers. Needs to be an integral type.
* @tparam weight_t Type of edge weights. Needs to be a floating point type.
* @tparam edge_type_t Type of edge type identifiers. Needs to be an integral type.
* @tparam edge_time_t Type of edge time. Needs to be an integral type.
* @tparam store_transposed Flag indicating whether to use sources (if false) or destinations (if
* true) as major indices in storing edges using a 2D sparse matrix.
* @tparam multi_gpu Flag indicating whether template instantiation should target single-GPU (false)
* or multi-GPU (true).
*
Expand All @@ -441,39 +453,56 @@ decompress_to_edgelist(
* compute_gpu_id_from_ext_edge_endpoints_t to every edge should return the local GPU ID for this
* function to work (edges should be pre-shuffled).
* @param edgelist_dsts Vector of edge destination vertex IDs.
* @param edgelist_weights Vector of edge weights.
* @param edgelist_edge_ids Vector of edge ids
* @param edgelist_edge_types Vector of edge types
* @param edgelist_edge_start_times Vector of edge start times
* @param edgelist_edge_end_times Vector of edge end times
* @param reciprocal Flag indicating whether to keep (if set to `false`) or discard (if set to
* `true`) edges that appear only in one direction.
* @return Tuple of symmetrized sources, destinations, optional weights, optional edge ids, optional
* edge types, optional edge start times and optional edge end times
* @param edgelist_edge_properties Vector of edge properties.
* @param edge_property_combining_rules Vector of rules for combining edge properties when adding
* reciprocal edges.
* @param store_transposed Flag indicating whether to use sources (if false) or destinations (if
* true) as major indices in storing edges using a 2D sparse matrix.
* @return Tuple of symmetrized sources, destinations, and edge properties.
*/
template <typename vertex_t,
typename edge_t,
typename weight_t,
typename edge_type_t,
typename edge_time_t,
bool store_transposed,
bool multi_gpu>
template <typename vertex_t, bool multi_gpu>
std::tuple<rmm::device_uvector<vertex_t>,
rmm::device_uvector<vertex_t>,
std::optional<rmm::device_uvector<weight_t>>,
std::optional<rmm::device_uvector<edge_t>>,
std::optional<rmm::device_uvector<edge_type_t>>,
std::optional<rmm::device_uvector<edge_time_t>>,
std::optional<rmm::device_uvector<edge_time_t>>>
symmetrize_edgelist(raft::handle_t const& handle,
rmm::device_uvector<vertex_t>&& edgelist_srcs,
rmm::device_uvector<vertex_t>&& edgelist_dsts,
std::optional<rmm::device_uvector<weight_t>>&& edgelist_weights,
std::optional<rmm::device_uvector<edge_t>>&& edgelist_edge_ids,
std::optional<rmm::device_uvector<edge_type_t>>&& edgelist_edge_types,
std::optional<rmm::device_uvector<edge_time_t>>&& edgelist_edge_start_times,
std::optional<rmm::device_uvector<edge_time_t>>&& edgelist_edge_end_times,
bool reciprocal);
std::vector<arithmetic_device_uvector_t>>
add_reciprocal_edges(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this name is confusing.

This sounds like we are adding reciprocal edges to another edge list. But isn't this inspecting the input edge list and keeping only the reciprocal edges?

I think we need a clearer name.

And the function below...

Isn't removing non-reciprocal edges same to the previous symmetrize_edgelist function with reciprocal = true. The comment is saying the opposite.

So, there are two ways to symmetrize.

  1. Only keep the reciprocal edges. Remove all non-reciprocal edges.
  2. Add inverse edges for all non-reciprocal edges.

I think we'd better rename the functions to make this more intuitive...

I asked this to ChatGPT, and these are the names ChatGPT suggested...

make_reciprocal_edges() <= But I think these names are bad... Still confusing.
make_symmetric_edges()

symmetrize_edgelist_union() <= Sounds better... not sure reciprocal & intersection aligns 100%
symmetrize_edgelist_intersection()

keep_reciprocal_edges() <= If I need to pick one out of the three.. I may go with this.
add_inverse_edges()

But there might be better names than this... Just a starting point...

raft::handle_t const& handle,
rmm::device_uvector<vertex_t>&& edgelist_srcs,
rmm::device_uvector<vertex_t>&& edgelist_dsts,
std::vector<arithmetic_device_uvector_t>&& edgelist_edge_properties,
std::vector<edge_property_combining_rule_t> const& edge_property_combining_rules,
bool store_transposed);

/**
* @ingroup graph_functions_cpp
* @brief Remove non-reciprocal edges from the edgelist. This was formerly called
* symmetrize_edgelist with reciprocal set to false.
*
* Non-reciprocal edges are edges that appear only in one direction. They will be removed from the
* edge list so that the resulting edge list is symmetric.
*
* @tparam vertex_t Type of vertex identifiers. Needs to be an integral type.
* @tparam multi_gpu Flag indicating whether template instantiation should target single-GPU (false)
* or multi-GPU (true).
* @param handle RAFT handle object to encapsulate resources (e.g. CUDA stream, communicator, and
* handles to various CUDA libraries) to run graph algorithms.
* @param edgelist_srcs Vector of edge source vertex IDs. If multi-GPU, applying the
* compute_gpu_id_from_ext_edge_endpoints_t to every edge should return the local GPU ID for this
* function to work (edges should be pre-shuffled).
* @param edgelist_dsts Vector of edge destination vertex IDs.
* @param edgelist_edge_properties Vector of edge properties.
* @param store_transposed Flag indicating whether to use sources (if false) or destinations (if
* true) as major indices in storing edges using a 2D sparse matrix.
* @return Tuple of non-reciprocal sources, destinations, and edge properties.
*/
template <typename vertex_t, bool multi_gpu>
std::tuple<rmm::device_uvector<vertex_t>,
rmm::device_uvector<vertex_t>,
std::vector<arithmetic_device_uvector_t>>
remove_nonreciprocal_edges(raft::handle_t const& handle,
rmm::device_uvector<vertex_t>&& edgelist_srcs,
rmm::device_uvector<vertex_t>&& edgelist_dsts,
std::vector<arithmetic_device_uvector_t>&& edgelist_edge_properties,
bool store_transposed);

/**
* @ingroup graph_functions_cpp
Expand Down