[API] create a graph with additional edge properties

[seunghwak]

Partially address #2479

We currently support only edge weight as edge properties. This PR's goal is to update cuGraph to support additional edge properties (currently only edge ID and type, eventually any arithmetic types or thrust tuple of arithmetic types).

This PR defines an API. There will be a separate PR for implementation.

[seunghwak]

rerun tests

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.10@cc05758). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.10    #2521   +/-   ##
===============================================
  Coverage                ?   61.11%           
===============================================
  Files                   ?      106           
  Lines                   ?     5634           
  Branches                ?        0           
===============================================
  Hits                    ?     3443           
  Misses                  ?     2191           
  Partials                ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[naimnv]

It looks good and understandable to me.

[naimnv]

@seunghwak Not sure if it's a good idea if we merge this two functions into one and use flag to return mutable or immutable view?

auto mutable_or_immutable_view(bool mutable){

    using value_iterator = mutable? decltype(get_dataframe_buffer_begin(buffers_[0])):decltype(get_dataframe_buffer_cbegin(buffers_[0]));

    std::vector<value_iterator> edge_partition_value_firsts(buffers_.size());
    std::vector<edge_type> edge_partition_edge_counts(buffers_.size());
    for (size_t i = 0; i < edge_partition_value_firsts.size(); ++i) {
      edge_partition_value_firsts[i] = mutable?get_dataframe_buffer_begin(buffers_[i]): get_dataframe_buffer_cbegin(buffers_[i]);
      edge_partition_edge_counts[i]  = size_dataframe_buffer(buffers_[i]);
    }
return detail::edge_property_view_t<edge_type, value_iterator>(
      edge_partition_value_firsts, edge_partition_edge_counts);
}
auto view() const{
    return mutable_or_immutable_view(false);
}
auto mutable_view(){
    return mutable_or_immutable_view(true);
}

[seunghwak]

This naming somewhat comes with a sort of RAPIDS convention (or I'd better say cuDF's naming scheme).

https://github.com/rapidsai/cudf/blob/branch-22.10/cpp/include/cudf/column/column_device_view.cuh#L928
and this convention somewhat leaked to cuCollection as well (https://github.com/NVIDIA/cuCollections/blob/dev/include/cuco/static_map.cuh#L772 not surprisingly as cuDF & cuCollection developers have an overlap).

I'm more inclined to stick with this convention unless there is a very strong reason to not follow this.

[naimnv]

I was rather thinking if we can have one common function with boolean flag to indicate constant view or mutable view. And depending on the boolean value the common function would return constant or mutable view.

`auto view() const{
return common_function(false);
}

auto mutable_view(){
return common_function(true);
}`

And that common function needs not to be exposed outside. The objective is to get rid of nearly duplicate code.

[seunghwak]

I guess the code above won't compile unless mutable is a non-type template parameter (so can be evaluated in compile time).

using value_iterator = mutable? decltype(get_dataframe_buffer_begin(buffers_[0])):decltype(get_dataframe_buffer_cbegin(buffers_[0]));

 edge_partition_value_firsts[i] = mutable?get_dataframe_buffer_begin(buffers_[i]): get_dataframe_buffer_cbegin(buffers_[i]);

some = A ? B : C;  // B and C should have the same type to compile.

Need to use std::conditional_t and if constexpr(mutalbe) but in this case, mutable can't be a run time variable.

If I create

template <bool mutable>
auto mutable_or_immutable_view() {
  ...
}

I guess the benefit in the binary size is gone. Not sure avoiding code duplication is worth the added additional complexity.

[naimnv]

I didn't think of that. Seems like we better merge as it is.

[naimnv]

Would edge_property_partition_device_view_t read better?

[seunghwak]

So, there is edge_partition_endpoint_property_device_view_t as well.

This name implies a device view object for edge properties in an edge partition (and edge_partition_endpoint_property_device_view_t means a device view object for edge endpoint (source/destination) properties in an edge partition).

[seunghwak]

And there will be vertex_partition_vertex_property_device_view_t in the future as well (to support vertex masking)... These names should be considered in this context.

edge_partition or vertex_partition comes first to emphasize that these work on a vertex partition or an edge partition.

"edge_property_partition" or "edge_endpoint_partition" somewhat sounds like we are partitioning edge properties or edge endpoints instead of edges... Edge partitioning precedes and edge properties or endpoint properties just follow this edge partition.

Based on this assumption, let me know if you have suggestions for better names.

[naimnv]

According the what you described, it's probably best to adhere with this naming convention.

[ChuckHastings]

@gpucibot merge