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

Add support in C API for handling unweighted graphs in algorithms that expect weights #3513

Merged

Conversation

ChuckHastings
Copy link
Collaborator

Closes #3499

Some of our C++ algorithms require a graph to be weighted. The python layer will add "fake" edge weights of 1 to every edge in order to call these algorithms. This adds a persisted constant value for every edge that will continue to exist for the life span of the python graph object, even though it is only necessary for a handful of algorithms.

This PR addresses these concerns.

Many of the C++ algorithms already actually support the graph not having weights. A few should support it and had bugs that were corrected (induced subgraph, k-core, egonet). A few can easily support it but did not (legacy spectral algorithms).

The Louvain and Leiden algorithms need to have a weighted graph because they will coarsen the weights as the graph is coarsened, and the weights will eventually not be one while the algorithm is running. These require a slightly different approach.

This PR also added C API tests for unweighted graphs for these algorithms.

@ChuckHastings ChuckHastings self-assigned this Apr 25, 2023
@ChuckHastings ChuckHastings added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 25, 2023
@ChuckHastings ChuckHastings added this to the 23.06 milestone Apr 25, 2023
@ChuckHastings ChuckHastings marked this pull request as ready for review April 25, 2023 20:16
@ChuckHastings ChuckHastings requested a review from a team as a code owner April 25, 2023 20:16
Comment on lines 130 to 148
template <typename GraphViewType, typename T>
auto create_constant_edge_property(raft::handle_t const& handle,
GraphViewType const& graph_view,
T constant_value)
{
edge_property_t<GraphViewType, T> edge_property(handle, graph_view);

auto mutable_view = edge_property.mutable_view();

std::for_each(
thrust::make_zip_iterator(mutable_view.value_firsts().begin(),
mutable_view.edge_counts().begin()),
thrust::make_zip_iterator(mutable_view.value_firsts().end(), mutable_view.edge_counts().end()),
[&handle, constant_value](auto tuple) {
detail::scalar_fill(handle, thrust::get<0>(tuple), thrust::get<1>(tuple), constant_value);
});

return edge_property;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We have fill_edge_src|dst_property() (https://github.com/rapidsai/cugraph/blob/branch-23.06/cpp/src/prims/fill_edge_src_dst_property.cuh).

So, it will be more consistent to create fill_edge_property() and call this instead of adding this function. I can quickly write fill_edge_property() if you agree.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Merged your PR and then my change to use it.

@ChuckHastings ChuckHastings requested a review from a team as a code owner April 27, 2023 16:04
Copy link
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

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

Better delete the no longer relevant include statement.

LGTM otherwise.

@@ -16,6 +16,7 @@

#pragma once

#include <cugraph/detail/utility_wrappers.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this include still necessary?

@BradReesWork
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 0fd72f9 into rapidsai:branch-23.06 May 4, 2023
rapids-bot bot pushed a commit that referenced this pull request May 9, 2023
@ChuckHastings ChuckHastings deleted the capi_handle_default_weights branch September 27, 2023 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle dummy weights in C API
3 participants