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 Bipartite Betweenness Centrality #32

Merged
merged 7 commits into from
Dec 5, 2024

Conversation

nv-rliu
Copy link
Contributor

@nv-rliu nv-rliu commented Nov 18, 2024

This PR adds bipartite > betweenness_centrality to nx-cugraph (seen here in NetworkX)

This was a combined effort with @eriknw to get myself familiarized with adding new algorithms.

@nv-rliu nv-rliu requested a review from a team as a code owner November 18, 2024 20:45
@nv-rliu nv-rliu requested a review from eriknw November 18, 2024 20:45
@eriknw eriknw added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Nov 18, 2024
Copy link
Contributor

@eriknw eriknw left a comment

Choose a reason for hiding this comment

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

It always makes me nervous when things "just work" on the first try 😂, but this all looks good to me, including 100% coverage of the new function.

We could consider adding k= and seed= as extra parameters or upstream to networkx (better than extra parameters), but I think it's probably okay to wait until somebody asks for them.

@eriknw eriknw added this to the 24.12 milestone Nov 19, 2024
@eriknw
Copy link
Contributor

eriknw commented Nov 22, 2024

I added a basic benchmark using a random graph.

Running on my desktop (I don't know how it compares to other machines):

---------------------------------------------------------------------- benchmark: 3 tests ---------------------------------------------------------------------
Name (time in s)                                                              Min                 Max                Mean            StdDev            Outliers
---------------------------------------------------------------------------------------------------------------------------------------------------------------
bench_bipartite_BC_n1000_m3000_k100000[backend=cugraph]                   20.8581 (1.0)       20.9796 (1.0)       20.9267 (1.0)      0.0623 (1.0)           1;0
bench_bipartite_BC_n1000_m3000_k100000[backend=cugraph-preconverted]      21.0158 (1.01)      21.3419 (1.02)      21.1412 (1.01)     0.1756 (2.82)          1;0
bench_bipartite_BC_n1000_m3000_k100000[backend=None]                     156.5447 (7.51)     168.9425 (8.05)     162.9700 (7.79)     6.2113 (99.74)         1;0
---------------------------------------------------------------------------------------------------------------------------------------------------------------

For nx-cugraph, >99% of the time is spent in plc.betweenness_centrality, which is the best we can hope for.

@eriknw
Copy link
Contributor

eriknw commented Nov 22, 2024

Companion PR that updates docs: rapidsai/cugraph#4778

@eriknw eriknw removed this from the 24.12 milestone Dec 2, 2024
Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

The enchmarks looks good.

@rlratzel
Copy link
Contributor

rlratzel commented Dec 5, 2024

/merge

@rapids-bot rapids-bot bot merged commit 138801c into rapidsai:branch-24.12 Dec 5, 2024
32 checks passed
AyodeAwe pushed a commit to rapidsai/cugraph that referenced this pull request Dec 5, 2024
@nv-rliu nv-rliu deleted the b24.12-bipartite-bc branch December 6, 2024 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmarks improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants