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 max_level= argument to louvain_communities to limit macro-iterations #6909

Merged
merged 4 commits into from Feb 15, 2024

Conversation

eriknw
Copy link
Contributor

@eriknw eriknw commented Sep 6, 2023

Louvain in cugraph allows max_level to be specified, which sets the upper limit of "macro iterations" of the Louvain algorithms (and other algorithms such as Leiden), so I thought it would be nice to upstream back to networkx.

Also, I changed the variable name of the iterator from d to it. d sounds like a dict to me.

TODO:

  • add to docstring

CC @rlratzel

@eriknw eriknw marked this pull request as draft September 7, 2023 01:50
Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

This seems like a nice addition to me. @z3y50n any thoughts on this one?

networkx/algorithms/community/louvain.py Outdated Show resolved Hide resolved
@eriknw eriknw marked this pull request as ready for review February 15, 2024 01:54
@eriknw
Copy link
Contributor Author

eriknw commented Feb 15, 2024

Updated docstring and responded to feedback. Ready for final review.

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks @eriknw

Technically, the introduction of the new kwarg before the end of the signature could bite users relying on kwarg position to call the function, but I don't think that's very likely.

The safest thing to do would be to add max_level as kwarg-only, but I don't feel too strongly about it!

@dschult dschult merged commit d709049 into networkx:main Feb 15, 2024
41 checks passed
@jarrodmillman jarrodmillman added this to the 3.3 milestone Feb 15, 2024
rapids-bot bot pushed a commit to rapidsai/cugraph that referenced this pull request Feb 26, 2024
…4177)

We already supported `max_level=`, and this was just upstreamed to networkx here: networkx/networkx#6909

Authors:
  - Erik Welch (https://github.com/eriknw)

Approvers:
  - Rick Ratzel (https://github.com/rlratzel)

URL: #4177
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
…rations (networkx#6909)

* Add `max_level=` argument to `louvain_communities` to limit macro-iterations

* ...or None

* Add docstring and update variable names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants