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

[BUG] Update some clustering algos to only support undirected graphs #2267

Conversation

jnke2016
Copy link
Contributor

Our current implementation of Louvain, Leiden, KTruss and Triangle Count only support undirected graphs. To enforce that, we are currently raising an exception if a graph is not of type Graph but this is ineffective if a user create a cugraph.Graph with the parameter directed=True

This PR

  • Raises an exception if directed=True when creating a cugraph.Graph
  • Update the docstrings for those algos
  • Add tests

closes #2263

@codecov-commenter
Copy link

codecov-commenter commented May 11, 2022

Codecov Report

Merging #2267 (3f1d7ef) into branch-22.06 (4a6263a) will decrease coverage by 40.09%.
The diff coverage is 0.00%.

@@                Coverage Diff                @@
##           branch-22.06    #2267       +/-   ##
=================================================
- Coverage         63.97%   23.87%   -40.10%     
=================================================
  Files               100      100               
  Lines              4436     4435        -1     
=================================================
- Hits               2838     1059     -1779     
- Misses             1598     3376     +1778     
Impacted Files Coverage Δ
...ython/cugraph/cugraph/community/ktruss_subgraph.py 20.58% <0.00%> (-64.71%) ⬇️
python/cugraph/cugraph/community/leiden.py 25.00% <0.00%> (-67.31%) ⬇️
python/cugraph/cugraph/community/louvain.py 25.00% <0.00%> (-67.31%) ⬇️
python/cugraph/cugraph/community/triangle_count.py 37.50% <0.00%> (-51.39%) ⬇️
python/cugraph/cugraph/dask/community/louvain.py 24.24% <0.00%> (-1.57%) ⬇️
python/cugraph/cugraph/components/connectivity.py 13.33% <0.00%> (-81.67%) ⬇️
python/cugraph/cugraph/traversal/sssp.py 10.67% <0.00%> (-81.56%) ⬇️
python/cugraph/cugraph/structure/hypergraph.py 8.46% <0.00%> (-81.54%) ⬇️
...thon/cugraph/cugraph/centrality/katz_centrality.py 7.84% <0.00%> (-80.40%) ⬇️
python/cugraph/cugraph/link_analysis/pagerank.py 16.00% <0.00%> (-80.00%) ⬇️
... and 41 more

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 4a6263a...3f1d7ef. Read the comment docs.

@BradReesWork BradReesWork added this to the 22.06 milestone May 11, 2022
@BradReesWork BradReesWork added bug Something isn't working 3 - Ready for Review non-breaking Non-breaking change labels May 11, 2022
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.

Looks good, just a few minor requests.

python/cugraph/cugraph/community/ktruss_subgraph.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/tests/mg/test_mg_louvain.py Outdated Show resolved Hide resolved
@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit da3911c into rapidsai:branch-22.06 May 17, 2022
@jnke2016 jnke2016 deleted the branch-22.06_support_undirected_graph_only_for_clustering_algo branch September 24, 2022 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[QST] Result of Leiden algorithm being different for renumbered and non-renumbered graph
4 participants