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

Labeling algorithm updates for C API #2185

Merged
merged 10 commits into from
Apr 13, 2022

Conversation

ChuckHastings
Copy link
Collaborator

@ChuckHastings ChuckHastings commented Apr 1, 2022

Several things bundled here:

  • Added C API definition, implementation and tests for weakly connected components
  • Added C API definition for strongly connected components
  • Updated the C API test functions for supporting the is_symmetric property
  • Added C unit tests for SCC, and an implementation that returns NOT_IMPLEMENTED

I could add some mock data returns, although the output from SCC is the same as the output from WCC. So python testing could just call WCC instead until there is an SCC implementation.

 1) Add C API definition/implementation/testing for WCC
 2) Add C API definition for SCC
 3) Update test utilities to support symmetric graph property
@ChuckHastings ChuckHastings added 2 - In Progress improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 1, 2022
@ChuckHastings ChuckHastings added this to the 22.06 milestone Apr 1, 2022
@ChuckHastings ChuckHastings marked this pull request as ready for review April 6, 2022 14:23
@ChuckHastings ChuckHastings requested review from a team as code owners April 6, 2022 14:23
if constexpr (!cugraph::is_candidate<vertex_t, edge_t, weight_t>::value) {
unsupported();
} else {
// WCC expects store_transposed == false
Copy link
Contributor

Choose a reason for hiding this comment

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

WCC=>SCC

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in latest push

struct wcc_functor : public cugraph::c_api::abstract_functor {
raft::handle_t const& handle_;
cugraph::c_api::cugraph_graph_t* graph_;
bool do_expensive_check_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add "{}"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In latest push

struct scc_functor : public cugraph::c_api::abstract_functor {
raft::handle_t const& handle_;
cugraph::c_api::cugraph_graph_t* graph_;
bool do_expensive_check_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add "{}"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in latest push

TRUE,
alpha,
epsilon,
max_iterations);
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the updates in this file from the eigenvector C API PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to modify the call to create_mg_test_graph and create_test_graph (the SG version) to add the is_symmetric parameter. This required me to change all of the C unit tests. Not quite sure why the indentation changed. Perhaps an upstream change in the clang format parameters?

@codecov-commenter
Copy link

codecov-commenter commented Apr 7, 2022

Codecov Report

Merging #2185 (75c7738) into branch-22.06 (38be932) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 75c7738 differs from pull request most recent head 644401c. Consider uploading reports for the commit 644401c to get more accurate results

@@               Coverage Diff                @@
##           branch-22.06    #2185      +/-   ##
================================================
+ Coverage         70.82%   70.86%   +0.04%     
================================================
  Files               170      170              
  Lines             11036    11036              
================================================
+ Hits               7816     7821       +5     
+ Misses             3220     3215       -5     
Impacted Files Coverage Δ
python/cugraph/cugraph/structure/hypergraph.py 90.00% <100.00%> (ø)
.../cugraph/tests/test_edge_betweenness_centrality.py 84.93% <0.00%> (+0.60%) ⬆️
...graph/cugraph/tests/test_betweenness_centrality.py 83.01% <0.00%> (+0.62%) ⬆️
python/cugraph/cugraph/tests/test_graph_store.py 100.00% <0.00%> (+1.56%) ⬆️
python/cugraph/cugraph/tests/test_utils.py 75.55% <0.00%> (+4.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8a6c60...644401c. Read the comment docs.

@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 5b5cd90 into rapidsai:branch-22.06 Apr 13, 2022
@ChuckHastings ChuckHastings deleted the fea_add_wcc_to_CAPI branch August 4, 2022 18:26
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.

4 participants