Enable PLC algos to leverage the PLC graph #2682
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## branch-22.10 #2682 +/- ##
===============================================
Coverage ? 60.04%
===============================================
Files ? 111
Lines ? 6184
Branches ? 0
===============================================
Hits ? 3713
Misses ? 2471
Partials ? 0 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. |
alexbarghi-nv
added
improvement
|
|
||
|
|
||
| def eigenvector_centrality( | ||
| G, max_iter=100, tol=1.0e-6, normalized=True | ||
| G, max_iter=100, tol=1.0e-6 |
There was a problem hiding this comment.
Removing the normalized arg would technically be a breaking change, even though it appears to never have been used. We should have a discussion if these should be removed (at which point the docstring probably shouldn't mention normalized either), or if we need to maintain them for keeping a backwards-compatible signature.
| @@ -92,7 +90,7 @@ def katz_centrality( | |||
| nstart['values'] : cudf.Series | |||
| Contains the katz centrality values of vertices | |||
|
|
|||
| normalized : bool, optional, default=True | |||
| normalized : not supported | |||
There was a problem hiding this comment.
Same for this one, although this appears to still be present as an optional arg to this function.
Edit: looks like there's a few others like this too.
There was a problem hiding this comment.
Right. I still don't get why they were added in the first place since they were never supported.
| @@ -151,7 +151,7 @@ def test_core_number_invalid_input(input_expected_output): | |||
| dg = cugraph.Graph(directed=True) | |||
| dg.from_dask_cudf_edgelist( | |||
| ddf, source='src', destination='dst', | |||
| edge_attr="value", renumber=True) | |||
| edge_attr="value", renumber=True, legacy_renum_only=True) | |||
There was a problem hiding this comment.
I'm a little worried about this change. What's the UX if legacy_renum_only=True is not specified? If not specifying this results in an error, then we'll need to figure out how to communicate that the user needs to specify this.
|
rerun tests |
|
@gpucibot merge |
This PR enables the PLC
algosto leverage thePLCgraph created prior to running thealgos. This will enable the exclusion of the graph creation time from the algo's run time for more accurate benchmarks. This PR also updates the tests accordingly (by skipping thecython.curenumbering) along with the docstrings.closes #2375
closes #2374