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

Define API for Betweenness Centrality #2823

Merged

Conversation

ChuckHastings
Copy link
Collaborator

Defines the C API, C++ API, C Unit tests and C++ unit tests for both SG and MG betweenness centrality.

Closes #2642
Closes #2643
Closes #2644
Closes #2646

@ChuckHastings ChuckHastings requested review from a team as code owners October 18, 2022 17:25
@ChuckHastings ChuckHastings self-assigned this Oct 18, 2022
@ChuckHastings ChuckHastings added 2 - In Progress improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 18, 2022
@ChuckHastings ChuckHastings added this to the 22.12 milestone Oct 18, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2022

Codecov Report

Base: 60.48% // Head: 60.28% // Decreases project coverage by -0.19% ⚠️

Coverage data is based on head (f017111) compared to base (fbd5f20).
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff                @@
##           branch-22.12    #2823      +/-   ##
================================================
- Coverage         60.48%   60.28%   -0.20%     
================================================
  Files               111      111              
  Lines              6526     6526              
================================================
- Hits               3947     3934      -13     
- Misses             2579     2592      +13     
Impacted Files Coverage Δ
...n/cugraph/cugraph/experimental/datasets/dataset.py 80.89% <0.00%> (-14.61%) ⬇️
python/cugraph/cugraph/structure/property_graph.py 95.44% <0.00%> (ø)

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

* betweenness either by random sampling @p num_vertices as the seeds of the traversal, or
* by using the provided @p vertices as the seeds of the traversal.
*
* If both @p and @num_vertices are specified we will throw an error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we better use std::optional<std::variant>>? Then, the code self documents the expected behavior and this comment is unnecessary.

* handles to various CUDA libraries) to run graph algorithms.
* @param graph_view Graph view object.
* @param num_vertices Optional, if specified, how many vertices to randomly select
* @param vertices Optional, if specified this provides the list of vertices to app
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like some texts after to app got deleted.

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 missing text, cleaned up documentation based on using std::variant

* betweenness either by random sampling @p num_vertices as the seeds of the traversal, or
* by using the provided @p vertices as the seeds of the traversal.
*
* If both @p and @num_vertices are specified we will throw an error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we better use std::optional<std::variant>>? Then, the code self documents the expected behavior and this comment is unnecessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Didn't think of that. Updated in next push.

* handles to various CUDA libraries) to run graph algorithms.
* @param graph_view Graph view object.
* @param num_vertices Optional, if specified, how many vertices to randomly select
* @param vertices Optional, if specified this provides the list of vertices to app
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like some texts after to app got deleted.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in next push

@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 3620d80 into rapidsai:branch-22.12 Oct 25, 2022
@ChuckHastings ChuckHastings deleted the define_mg_betweenness_api branch December 2, 2022 18:35
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
4 participants