[WIP] Louvain algorithm for undirected graphs by jpacold · Pull Request #1277 · Qiskit/rustworkx

jpacold · 2024-09-05T04:08:43Z

I finally had some time to work on #1141, so far only on the Rust side and only for undirected graphs. Opening this for feedback on how the Rust code is organized, and maybe to restart the discussion on whether this should go here or in petgraph.

The big additions here are:

metrics.rs
- Modularity: a trait for graphs for which we can compute modularity (by extension this means we can apply the Louvain method)
- modularity: computes the modularity, given a graph and a set of subsets.
- Partition: a struct holding a graph partition. This mainly exists to keep the implementation of modularity organized.
louvain.rs
- InnerGraph: at each level of the algorithm we work with an aggregated graph, where each node of this graph corresponds to one of the communities that were identified in the previous level). InnerGraph keeps track of this correspondence.
- LouvainAlgo: helper functions for updating the Partition at each level of the algorithm.
- one_level_undirected: most of the business logic is in here since it constructs the new communities at each level, roughly equivalent to _one_level in the networkx implementation.
- louvain_communities: repeatedly calls one_level_undirected, stopping either after a fixed number of iterations or if the modularity improvement falls below the given threshold.

I realize this is a lot to review at once and can go into more detail about each piece if necessary.

…for partition instead of `Vec`

…ouvain

… cleanup

IvanIsCoding

Overall this is a great start! I would call it more than a draft given it already has good tests.

I left some Rust comments, I need to study the algorithm more myself to give more "algorithmic" advice

IvanIsCoding · 2024-09-08T15:11:45Z

+    }
+
+    #[test]
+    fn test_louvain_karate_club_graph() {


Friendly reminder to myself that we should add a Karate club graph generator. It could be a separate PR

Once #1280 gets merged this test will hopefully be much shorter

IvanIsCoding · 2024-09-08T15:13:23Z

+        // happen to get the same result as:
+        //      import networkx as nx
+        //      g = nx.karate_club_graph()
+        //      communities = nx.community.louvain_communities(g, weight='weight', seed=12)


I think this is a brittle test (https://testing.googleblog.com/2024/04/how-i-learned-to-stop-writing-brittle.html) but leave a TODO there and we can revisit it later. I am glad it passes though

I'm not sure yet what to do with this. I can adjust the resolution parameter so that we consistently get two output communities. However, there is still a lot of dependence on the seed (in networkx I get at least 5 different partitions with the same parameters)

IvanIsCoding · 2024-09-08T15:17:27Z

+        };
+
+        let m: f64 = total_edge_weight(self.graph);
+        sigma_internal / m - resolution * sigma_total_squared / (m * m)


We might want to reorganize this expression to avoid floating point erros, specially because of the common factor 1/m

I started doing this in 58857fd. Other than the common factor what are the possible issues? I think sigma_internal, sigma_total_squared / m, and m are all about the same order of magnitude but could be missing something.

…t node degrees

jpacold · 2024-09-10T03:58:30Z

Fixed two mistakes in the modularity gain calculation (I am actually a little surprised that the tests passed earlier and need to come up with some more)
Addressed some of the comments

It looks as if the CI is failing because I used associated type bounds.

IvanIsCoding · 2024-09-14T23:38:05Z

Fixed two mistakes in the modularity gain calculation (I am actually a little surprised that the tests passed earlier and need to come up with some more)

Addressed some of the comments

It looks as if the CI is failing because I used associated type bounds.

Indeed our current MSRV is Rust 1.70 from June 2023. Rust 1.79 is from June 2024. In the past we used to follow Debian stable's Rustc which believe it or not is in 1.63 still.

I'd try rewriting it before we discuss bumping MSR, 1.79 might break a couple people that install from source

coveralls · 2024-09-16T05:26:23Z

Pull Request Test Coverage Report for Build 10877937563

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 293 (0.0%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-1.5%) to 94.344%

Changes Missing Coverage	Changed/Added Lines	%
rustworkx-core/src/community/mod.rs	6	0.0%
rustworkx-core/src/community/utils.rs	10	0.0%
rustworkx-core/src/community/metrics.rs	81	0.0%
rustworkx-core/src/community/louvain.rs	196	0.0%

Totals
Change from base Build 10672676557:	-1.5%
Covered Lines:	17999
Relevant Lines:	19078

💛 - Coveralls

rustworkx upstream has no community detection algorithm at all (Louvain or Leiden) - confirmed via issue Qiskit#1141 (open since 2024-03, unclaimed) and its draft PR Qiskit#1277 (stale since 2024-09, never merged). Rather than writing Leiden from scratch, vendors it from network_partitions (MIT, part of graspologic-org/graspologic-native), re-verified live before use: petgraph interop support (feature = "petgraph"), MIT/Microsoft, actively maintained, not on crates.io so pulled as a git dependency pinned by commit in rustworkx-core/Cargo.toml. Required a newer Rust toolchain than this repo's own declared MSRV (1.85): network_partitions' 2024-edition source uses a let-chains pattern that only compiles on a newer stable release (tested with 1.98). rustworkx's own code is unaffected - this is purely a build requirement for the vendored dependency. rustworkx-core::community::leiden is generic over any IntoNodeIdentifiers + IntoEdgeReferences graph plus an edge weight_fn closure (matching this crate's existing weighted-algorithm convention, e.g. shortest_path's edge_cost). Its job is entirely adaptation: build a fresh, densely-indexed petgraph::graph::UnGraph<f64,f64> from the caller's graph (network_partitions' PetgraphNetworkView requires contiguous 0..n indices, which an arbitrary StableGraph with removed nodes wouldn't have), compute each node's weighted degree as its node weight (required for modularity mode - CPM mode gets 1.0 uniformly), run leiden_view(), then translate cluster assignments back to the caller's own node identifiers - never assumes network_partitions' internal indices mean anything outside this function. Exposed to Python as graph_leiden(graph, weight_fn=None, default_weight=1.0, resolution=1.0, iterations=2, randomness=None, use_modularity=True, seed=None) -> dict[int, int], following the same weight_fn/default_weight calling convention as the rest of this codebase's weighted algorithms (weight_callable/edge_weights_from_callable). Verified: rustworkx-core unit tests (two-clique split via the generic wrapper - not just network_partitions' own PetgraphNetworkView tests, to exercise this crate's actual conversion code; empty-graph and negative-weight error handling; same-seed determinism) and end-to-end Python tests (two-clique split, default-weight fallback without weight_fn, determinism, empty-graph/negative-weight errors, a complete graph collapsing to one community, an edgeless graph producing all singletons). Full existing test suites unaffected: rustworkx-core 373 Rust tests and 2,388 Python tests (11 pre-existing skips) all still pass.

jpacold added 26 commits July 3, 2024 21:55

create community module, stubs for modularity and helper functions

77746f5

implement _total_edge_weight

bd1ee6c

implement modularity function

6ada2b9

add Rust test, fix modularity calculation

75f2570

Merge branch 'Qiskit:main' into louvain

dc444d6

add test for directed case

cb7fd50

Merge branch 'Qiskit:main' into louvain

1f24463

Merge branch 'Qiskit:main' into louvain

561ab21

use associate type bounds to simplify where clauses, use HashSet …

b2f181c

…for partition instead of `Vec`

create struct Partition

3568a76

Move modularity calculation to Partition

b986a8e

Add struct for Louvain algorithm

0bcd747

Minimal working version for undirected graphs

fb3faea

Use random seed argument

ab976fb

Merge branch 'Qiskit:main' into louvain

5fcb89e

Merge branch 'louvain' of https://github.com/jpacold/rustworkx into l…

c75ec5f

…ouvain

Start factoring out helper functions, use NodeIndexable for some code…

e0fed65

… cleanup

Use Vec instead of HashMap where possible

2609291

Clean up Partition

12f5364

Avoid copying the whole input graph on the first pass

fda816c

Add comments

611d08a

Make InnerGraph an enum

885045c

Separate Louvain algorithm code from modularity code

7a2301c

Add karate club test

c0e2de3

Merge branch 'Qiskit:main' into louvain

df684f6

Remove redundant argument from one_level_undirected

b3fad58

IvanIsCoding reviewed Sep 8, 2024

View reviewed changes

jpacold added 3 commits September 8, 2024 20:21

Fix mistake in gain calculation: update current community degrees, no…

e399a8b

…t node degrees

Use degree of node instead of degree of community

ae3d19c

Update karate club test

bc976b4

jpacold added 4 commits September 8, 2024 20:55

Use closure for repeated part of gain calculation

12ffc33

Replace loop with while performed_move

234659e

Replace if let with match

11a74fd

Reorganize modularity and gain arithmetic

58857fd

IvanIsCoding mentioned this pull request Sep 14, 2024

Add Zachary's Karate Club #1280

Merged

jpacold added 2 commits September 15, 2024 23:11

Revise traits to compile with Rust 1.70

e77f71c

cargo fmt --all

6f49cae

This was referenced Jul 6, 2025

add more community detection algorithm as Louvain (blondel 2008) petgraph/petgraph#185

Open

feat: add Louvain community detection algorithm petgraph/petgraph#848

Closed

Navigation Menu

Uh oh!

[WIP] Louvain algorithm for undirected graphs - #1277

[WIP] Louvain algorithm for undirected graphs#1277
jpacold wants to merge 35 commits into
Qiskit:mainfrom
jpacold:louvain

jpacold commented Sep 5, 2024

Uh oh!

IvanIsCoding left a comment

Uh oh!

Uh oh!

Uh oh!

IvanIsCoding Sep 8, 2024

Uh oh!

IvanIsCoding Sep 9, 2024

Uh oh!

IvanIsCoding Sep 8, 2024

Uh oh!

jpacold Sep 10, 2024

Uh oh!

Uh oh!

IvanIsCoding Sep 8, 2024

Uh oh!

jpacold Sep 10, 2024

Uh oh!

jpacold commented Sep 10, 2024

Uh oh!

IvanIsCoding commented Sep 14, 2024

Uh oh!

coveralls commented Sep 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jpacold commented Sep 5, 2024

Uh oh!

IvanIsCoding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

IvanIsCoding Sep 8, 2024

Choose a reason for hiding this comment

Uh oh!

IvanIsCoding Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

IvanIsCoding Sep 8, 2024

Choose a reason for hiding this comment

Uh oh!

jpacold Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

IvanIsCoding Sep 8, 2024

Choose a reason for hiding this comment

Uh oh!

jpacold Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

jpacold commented Sep 10, 2024

Uh oh!

IvanIsCoding commented Sep 14, 2024

Uh oh!

coveralls commented Sep 16, 2024

Pull Request Test Coverage Report for Build 10877937563

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants