diff --git a/packages/cli/src/leiden.rs b/packages/cli/src/leiden.rs index b87610e..9c0ef69 100644 --- a/packages/cli/src/leiden.rs +++ b/packages/cli/src/leiden.rs @@ -61,7 +61,6 @@ pub fn leiden( &mut rng, use_modularity, None, - None, ); let leiden_completion_instant: Instant = Instant::now(); diff --git a/packages/network_partitions/src/interop/mod.rs b/packages/network_partitions/src/interop/mod.rs index 98e025e..87b1986 100644 --- a/packages/network_partitions/src/interop/mod.rs +++ b/packages/network_partitions/src/interop/mod.rs @@ -56,32 +56,12 @@ mod tests { let compact = CompactNetwork::from(nodes, neighbors, 0.0); let mut rng1 = SmallRng::seed_from_u64(42); - let (_improved1, c1) = leiden( - &compact, - None, - Some(1), - None, - None, - &mut rng1, - true, - None, - None, - ) - .unwrap(); + let (_improved1, c1) = + leiden(&compact, None, Some(1), None, None, &mut rng1, true, None).unwrap(); let mut rng2 = SmallRng::seed_from_u64(42); - let (_improved2, c2) = leiden_view( - &compact, - None, - Some(1), - None, - None, - &mut rng2, - true, - Some(10), - None, - ) - .unwrap(); + let (_improved2, c2) = + leiden_view(&compact, None, Some(1), None, None, &mut rng2, true, None).unwrap(); // Both should find 2 communities assert_eq!(c1.next_cluster_id(), 2); diff --git a/packages/network_partitions/src/interop/petgraph_interop.rs b/packages/network_partitions/src/interop/petgraph_interop.rs index 7b762be..0953920 100644 --- a/packages/network_partitions/src/interop/petgraph_interop.rs +++ b/packages/network_partitions/src/interop/petgraph_interop.rs @@ -421,7 +421,6 @@ mod tests { None, &mut rng, false, - Some(10), None, ) .unwrap(); @@ -471,18 +470,8 @@ mod tests { let view = PetgraphNetworkView::new(&g).unwrap(); let mut rng = SmallRng::seed_from_u64(42); - let (_improved, clustering) = leiden_view( - &view, - None, - Some(2), - Some(0.5), - None, - &mut rng, - false, - Some(10), - None, - ) - .unwrap(); + let (_improved, clustering) = + leiden_view(&view, None, Some(2), Some(0.5), None, &mut rng, false, None).unwrap(); // improved flag depends on resolution and graph structure // Should find 2 communities @@ -536,7 +525,6 @@ mod tests { None, &mut rng, false, - Some(10), None, ) .unwrap(); @@ -600,18 +588,8 @@ mod tests { let view = PetgraphNetworkView::new(&g).unwrap(); let mut rng = SmallRng::seed_from_u64(123); - let (_, clustering) = leiden_view( - &view, - None, - Some(2), - Some(0.5), - None, - &mut rng, - false, - Some(10), - None, - ) - .unwrap(); + let (_, clustering) = + leiden_view(&view, None, Some(2), Some(0.5), None, &mut rng, false, None).unwrap(); // Should split into 2 communities assert_eq!( @@ -658,18 +636,8 @@ mod tests { let view = PetgraphNetworkView::new(&g).unwrap(); let mut rng = SmallRng::seed_from_u64(42); - let (_, clustering) = leiden_view( - &view, - None, - Some(2), - Some(0.5), - None, - &mut rng, - false, - Some(10), - None, - ) - .unwrap(); + let (_, clustering) = + leiden_view(&view, None, Some(2), Some(0.5), None, &mut rng, false, None).unwrap(); // Each node should be in its own community (no reason to merge) assert_eq!(clustering.num_nodes(), 3); diff --git a/packages/network_partitions/src/interop/sprs_interop.rs b/packages/network_partitions/src/interop/sprs_interop.rs index 656c056..e8de074 100644 --- a/packages/network_partitions/src/interop/sprs_interop.rs +++ b/packages/network_partitions/src/interop/sprs_interop.rs @@ -361,18 +361,8 @@ mod tests { let view = SprsNetworkView::new(&mat).unwrap(); let mut rng = SmallRng::seed_from_u64(42); - let (_improved, clustering) = leiden_view( - &view, - None, - Some(2), - Some(0.5), - None, - &mut rng, - false, - Some(10), - None, - ) - .unwrap(); + let (_improved, clustering) = + leiden_view(&view, None, Some(2), Some(0.5), None, &mut rng, false, None).unwrap(); // improved flag depends on resolution and graph structure let communities: std::collections::HashSet = (0..clustering.num_nodes()) @@ -404,18 +394,8 @@ mod tests { let view = SprsNetworkView::new(&mat).unwrap(); let mut rng = SmallRng::seed_from_u64(42); - let (_improved, clustering) = leiden_view( - &view, - None, - Some(2), - Some(0.5), - None, - &mut rng, - false, - Some(10), - None, - ) - .unwrap(); + let (_improved, clustering) = + leiden_view(&view, None, Some(2), Some(0.5), None, &mut rng, false, None).unwrap(); // improved flag depends on resolution and graph structure let communities: std::collections::HashSet = (0..clustering.num_nodes()) @@ -460,18 +440,8 @@ mod tests { let view = SprsNetworkView::new(&mat).unwrap(); let mut rng = SmallRng::seed_from_u64(42); - let (_, clustering) = leiden_view( - &view, - None, - Some(2), - Some(0.5), - None, - &mut rng, - false, - Some(10), - None, - ) - .unwrap(); + let (_, clustering) = + leiden_view(&view, None, Some(2), Some(0.5), None, &mut rng, false, None).unwrap(); // Nodes 0 and 1 should be in the same community assert_eq!( @@ -487,18 +457,8 @@ mod tests { let view = SprsNetworkView::new(&mat).unwrap(); let mut rng = SmallRng::seed_from_u64(42); - let (_, clustering) = leiden_view( - &view, - None, - Some(2), - Some(0.5), - None, - &mut rng, - false, - Some(10), - None, - ) - .unwrap(); + let (_, clustering) = + leiden_view(&view, None, Some(2), Some(0.5), None, &mut rng, false, None).unwrap(); // Each pair should form its own community assert_eq!( @@ -611,7 +571,6 @@ mod tests { None, &mut rng, false, - Some(10), None, ) .unwrap(); @@ -639,7 +598,6 @@ mod tests { None, &mut rng, false, - Some(10), None, ) .unwrap(); diff --git a/packages/network_partitions/src/leiden/hierarchical.rs b/packages/network_partitions/src/leiden/hierarchical.rs index 0ce0873..48437af 100644 --- a/packages/network_partitions/src/leiden/hierarchical.rs +++ b/packages/network_partitions/src/leiden/hierarchical.rs @@ -125,7 +125,6 @@ pub fn hierarchical_leiden( rng: &mut T, use_modularity: bool, max_cluster_size: u32, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> Result, CoreError> where @@ -139,7 +138,6 @@ where randomness, rng, use_modularity, - max_outer_iterations, max_local_moving_iterations, )?; @@ -174,7 +172,6 @@ where randomness, rng, use_modularity, - max_outer_iterations, max_local_moving_iterations, )?; let offset: usize = updated_clustering.next_cluster_id(); diff --git a/packages/network_partitions/src/leiden/leiden_clustering.rs b/packages/network_partitions/src/leiden/leiden_clustering.rs index 5405be3..a403e66 100644 --- a/packages/network_partitions/src/leiden/leiden_clustering.rs +++ b/packages/network_partitions/src/leiden/leiden_clustering.rs @@ -16,44 +16,40 @@ use super::subnetwork::SubnetworkClusteringGenerator; const DEFAULT_ITERATIONS: usize = 1; -/// Improves a clustering by performing `iterations` of the Leiden algorithm, which is itself -/// a recursive algorithm. +/// Performs the Leiden community detection algorithm on a `CompactNetwork`. /// -/// The Leiden algorithm consists of three phases: -/// - local moving of nodes between clusters -/// - refinement of the clusters -/// - aggregation of the network based on the refined clusters, using the non-refined clusters to -/// create an initial clustering for the aggregate network +/// Each outer iteration runs the full Leiden cycle on the original network: +/// 1. **Local moving** — greedily moves nodes between clusters to maximize the +/// quality function. +/// 2. **Refinement** — stochastically splits each cluster into sub-clusters to +/// escape local optima. +/// 3. **Aggregation** — builds an induced (coarsened) network from the refined +/// clustering and recursively repeats steps 1–3 on it until the network +/// stops shrinking (inner convergence). +/// 4. **Mapping back** — projects the coarsened clustering onto the original nodes. /// -/// These phases are repeated until no further improvements can be made. +/// Because these phases include randomness, running multiple outer iterations +/// gives the algorithm additional chances to escape suboptimal partitions. /// -/// Because these phases include a random number generator, `iterations` acts as a further -/// refinement of the process, ensuring that we do at least `iterations-1` further tries to ensure -/// that we've actually achieved a stable partitioning. +/// # Parameters /// -/// If an initial Clustering is provided, it will be used as the starting point for the Leiden algorithm, -/// otherwise each node will be placed in their own cluster to start. -/// -/// network: InternalNetwork to generate or update a clustering for based on the Leiden algorithm -/// clustering: An optional initial clustering. If an initial Clustering is provided, it will be used -/// as the starting point for the Leiden algorithm, otherwise each node will be placed in their own -/// cluster to start. -/// iterations: The leiden algorithm is recursive and will continue until improvements cannot be made; however, -/// randomization is a part of the algorithm and you may request further iterations by setting iterations -/// to be a number greater than 1 to force it to try a few more times for some minor, further refinements. -/// resolution: Default is 1.0, and impacts the maximization function used. The resolution must be greater than -/// zero. A higher resolution values leads to more communities, a lower resolution parameter leads to fewer -/// communities. -/// randomness: Default is 1E-2. The value must be greater than 0. The higher the randomness value, the more -/// exploration of the partition space is possible. This is a major difference from the Louvain algorithm. -/// The Louvain algorithm is purely greedy in the partition exploration. -/// seed: If a seed is provided, the Pseudo-Random Number Generator will be created using that seed. -/// Useful for replicating results between runs. -/// use_modularity: Leiden uses a maximization function, and this lets you specify whether you wish -/// to use modularity or Constant Potts Model (CPM). It's vital that the InternalNetwork is appropriate -/// for this setting: see InternalNetwork::for_modularity_maximization or -/// InternalNetwork::for_cpm_maximization and ensure you use the function that builds the corresponding -/// InternalNetwork for this setting. +/// - `network`: The network to cluster. +/// - `clustering`: Optional initial clustering. If `None`, each node starts in +/// its own singleton cluster. +/// - `iterations`: Number of outer iterations (default 1). Each iteration +/// re-runs the full cycle on the original network, using the previous +/// clustering as the starting point. This refines a single solution +/// progressively (distinct from `trials`, which is implemented at the +/// Python layer and runs independent attempts keeping the best). +/// - `resolution`: Quality-function resolution (default 1.0, must be > 0). +/// Higher values produce more communities; lower values produce fewer. +/// - `randomness`: Controls exploration during refinement (default 1e-2, must +/// be > 0). Higher values allow more exploration of the partition space. +/// - `rng`: A seeded random number generator for reproducibility. +/// - `use_modularity`: If `true`, optimizes modularity; if `false`, uses CPM. +/// The network must be constructed appropriately for the chosen mode. +/// - `max_local_moving_iterations`: Limits the number of node-processing sweeps +/// within a single local-moving call. `None` or `Some(0)` means unlimited. pub fn leiden( network: &CompactNetwork, clustering: Option, @@ -62,7 +58,6 @@ pub fn leiden( randomness: Option, rng: &mut T, use_modularity: bool, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> Result<(bool, Clustering), CoreError> where @@ -70,7 +65,6 @@ where { let iterations: usize = iterations.unwrap_or(DEFAULT_ITERATIONS); let randomness: f64 = randomness.unwrap_or(subnetwork::DEFAULT_RANDOMNESS); - let max_outer: u32 = max_outer_iterations.unwrap_or(0); let max_local: u32 = max_local_moving_iterations.unwrap_or(0); let adjusted_resolution: f64 = adjust_resolution(resolution, network, use_modularity); @@ -96,7 +90,6 @@ where adjusted_resolution, randomness, rng, - max_outer, max_local, )?; } @@ -104,12 +97,13 @@ where Ok((improved, clustering)) } -/// Like `leiden`, but operates on any `NetworkView` implementation for zero-copy support. +/// Like [`leiden`], but operates on any [`NetworkView`] implementation for zero-copy support. /// -/// The initial local-moving phase runs directly on the provided view (zero-copy). -/// If the algorithm needs to recurse (aggregation), it materializes a `CompactNetwork` -/// internally. This gives the best of both worlds: zero-copy for the expensive first -/// pass, with full recursive support when needed. +/// The first local-moving pass runs directly on the provided view (avoiding +/// materialization). If aggregation is needed, a `CompactNetwork` is created +/// internally for the recursive phases. This gives the best of both worlds: +/// zero-copy for the expensive initial pass on large external data structures, +/// with full recursive aggregation when needed. pub fn leiden_view( network: &N, clustering: Option, @@ -118,7 +112,6 @@ pub fn leiden_view( randomness: Option, rng: &mut T, use_modularity: bool, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> Result<(bool, Clustering), CoreError> where @@ -127,7 +120,6 @@ where { let iterations: usize = iterations.unwrap_or(DEFAULT_ITERATIONS); let randomness: f64 = randomness.unwrap_or(subnetwork::DEFAULT_RANDOMNESS); - let max_outer: u32 = max_outer_iterations.unwrap_or(0); let max_local: u32 = max_local_moving_iterations.unwrap_or(0); let adjusted_resolution: f64 = adjust_resolution(resolution, network, use_modularity); @@ -154,7 +146,6 @@ where adjusted_resolution, randomness, rng, - max_outer, max_local, )?; } @@ -162,7 +153,12 @@ where Ok((improved, clustering)) } -/// First improvement pass using a generic NetworkView, then materializes for recursion. +/// Single outer-iteration pass using a generic NetworkView. +/// +/// Runs local moving on the view (zero-copy), then — if clusters were formed — +/// materializes a CompactNetwork and delegates to [`improve_clustering_recursive`] +/// for the refinement and aggregation phases. Inner recursion always runs to +/// convergence (no depth limit). fn improve_clustering_view( network: &N, clustering: &mut Clustering, @@ -170,7 +166,6 @@ fn improve_clustering_view( adjusted_resolution: f64, randomness: f64, rng: &mut T, - max_outer_iterations: u32, max_local_moving_iterations: u32, ) -> Result where @@ -186,18 +181,11 @@ where max_local_moving_iterations, )?; - if clustering.next_cluster_id() < network.num_nodes() - && (max_outer_iterations == 0 || max_outer_iterations > 1) - { - // Recursion needed: materialize to CompactNetwork and delegate + if clustering.next_cluster_id() < network.num_nodes() { + // Clusters were formed — materialize to CompactNetwork for refinement + // and recursive aggregation. let compact_network = network.to_compact_network(); - let next_max_outer = if max_outer_iterations == 0 { - 0 - } else { - max_outer_iterations - 1 - }; - improved |= improve_clustering_recursive( &compact_network, clustering, @@ -205,14 +193,22 @@ where adjusted_resolution, randomness, rng, - next_max_outer, max_local_moving_iterations, )?; } Ok(improved) } -/// Recursive aggregation phase — always operates on CompactNetwork. +/// Refinement and recursive aggregation phase — always operates on CompactNetwork. +/// +/// Given a clustering produced by local moving, this function: +/// 1. Refines each cluster via stochastic sub-clustering. +/// 2. Builds an induced (coarsened) network from the refined clustering. +/// 3. If the induced network is smaller, recursively calls [`improve_clustering`] +/// on it (which repeats LM → refine → aggregate until convergence). +/// 4. If the induced network is NOT smaller (no aggregation progress), runs one +/// final LM pass on it without further recursion to avoid infinite oscillation. +/// 5. Maps the induced-network clustering back onto the original nodes. fn improve_clustering_recursive( network: &CompactNetwork, clustering: &mut Clustering, @@ -220,7 +216,6 @@ fn improve_clustering_recursive( adjusted_resolution: f64, randomness: f64, rng: &mut T, - max_outer_iterations: u32, max_local_moving_iterations: u32, ) -> Result where @@ -248,7 +243,12 @@ where clustering.update_cluster_at(*singleton_node, clustering.next_cluster_id())?; num_nodes_per_cluster_induced_network.push(1); } else if item.subnetwork.num_nodes() == 0 { - panic!("No node network, which shouldn't have happened"); + // Multi-node cluster with no internal edges — split into singletons. + let cluster_nodes: &Vec = &nodes_by_cluster[item.id]; + for node in cluster_nodes { + clustering.update_cluster_at(*node, clustering.next_cluster_id())?; + num_nodes_per_cluster_induced_network.push(1); + } } else { let subnetwork_clustering: Clustering = subnetwork_clusterer.subnetwork_clustering( item.subnetwork.compact(), @@ -271,16 +271,30 @@ where ); let mut improved = false; - improved |= improve_clustering( - &induced_clustering_network, - &mut induced_network_clustering, - use_modularity, - adjusted_resolution, - randomness, - rng, - max_outer_iterations, - max_local_moving_iterations, - )?; + + if induced_clustering_network.num_nodes() < network.num_nodes() { + // Induced network is smaller — recurse to convergence. + improved |= improve_clustering( + &induced_clustering_network, + &mut induced_network_clustering, + use_modularity, + adjusted_resolution, + randomness, + rng, + max_local_moving_iterations, + )?; + } else { + // No shrinkage — run one final LM pass on the induced network + // (refinement may have split clusters that LM can re-merge) but + // don't recurse further to avoid infinite oscillation. + improved |= full_network_clustering::full_network_clustering( + &induced_clustering_network, + &mut induced_network_clustering, + adjusted_resolution, + rng, + max_local_moving_iterations, + )?; + } clustering.merge_clustering(&induced_network_clustering); Ok(improved) @@ -319,7 +333,11 @@ fn guarantee_clustering_sanity_view( Ok(()) } -/// This function will be executed repeatedly as per number_iterations +/// Single outer-iteration pass on a CompactNetwork. +/// +/// Runs local moving, then — if any nodes were merged — delegates to +/// [`improve_clustering_recursive`] for the refinement and aggregation phases. +/// Inner recursion always runs to convergence (no depth limit). fn improve_clustering( network: &CompactNetwork, clustering: &mut Clustering, @@ -327,13 +345,12 @@ fn improve_clustering( adjusted_resolution: f64, randomness: f64, rng: &mut T, - max_outer_iterations: u32, max_local_moving_iterations: u32, ) -> Result where T: Rng + Clone + Send, { - // do a slower, higher fidelity full network clustering + // Local moving: greedily reassign nodes to maximize the quality function let mut improved: bool = full_network_clustering::full_network_clustering( network, clustering, @@ -342,81 +359,16 @@ where max_local_moving_iterations, )?; - if clustering.next_cluster_id() < network.num_nodes() - && (max_outer_iterations == 0 || max_outer_iterations > 1) - { - // given the updated clustering, generate subnetworks for each cluster comprised solely of the - // nodes in that cluster, then fast, low-fidelity cluster the subnetworks, merging the results - // back into the primary clustering before returning - let nodes_by_cluster: Vec> = clustering.nodes_per_cluster(); - let subnetworks_iterator = network.subnetworks_iter(clustering, &nodes_by_cluster, None); - let num_nodes_per_cluster: Vec = clustering.num_nodes_per_cluster(); - - let num_subnetworks: usize = clustering.next_cluster_id(); - - clustering.reset_next_cluster_id(); - - let mut num_nodes_per_cluster_induced_network: Vec = - Vec::with_capacity(num_subnetworks); - let max_subnetwork_size: u64 = *num_nodes_per_cluster.iter().max().unwrap(); - let mut subnetwork_clusterer = - SubnetworkClusteringGenerator::with_capacity(max_subnetwork_size as usize); - - for item in subnetworks_iterator { - if num_nodes_per_cluster[item.id] == 1 && item.subnetwork.num_nodes() == 0 { - // this is a singleton cluster, and cannot move from what it previously was. - // the subnetwork actually has no information about the nodes in it, because we don't - // store nodes without neighbors in the network objects, so instead we need to ask the iterator - // for some internal state - let single_node_vec: &Vec = &nodes_by_cluster[item.id]; - // manually merge this into the clustering object with the right value - let singleton_node: &usize = single_node_vec - .first() - .expect("There should be one node here"); - clustering.update_cluster_at(*singleton_node, clustering.next_cluster_id())?; - num_nodes_per_cluster_induced_network.push(1); - } else if item.subnetwork.num_nodes() == 0 { - // this is a bug, and we should panic - panic!("No node network, which shouldn't have happened"); - } else { - let subnetwork_clustering: Clustering = subnetwork_clusterer - .subnetwork_clustering( - item.subnetwork.compact(), - use_modularity, - adjusted_resolution, - randomness, - rng, - )?; - num_nodes_per_cluster_induced_network.push(subnetwork_clustering.next_cluster_id()); - clustering.merge_subnetwork_clustering(&item.subnetwork, &subnetwork_clustering); - } - } - - let induced_clustering_network: CompactNetwork = - network.induce_clustering_network(clustering)?; - - let mut induced_network_clustering = initial_clustering_for_induced( - num_nodes_per_cluster_induced_network, - induced_clustering_network.num_nodes(), - ); - - let next_max_outer = if max_outer_iterations == 0 { - 0 - } else { - max_outer_iterations - 1 - }; - - improved |= improve_clustering( - &induced_clustering_network, - &mut induced_network_clustering, + if clustering.next_cluster_id() < network.num_nodes() { + improved |= improve_clustering_recursive( + network, + clustering, use_modularity, adjusted_resolution, randomness, rng, - next_max_outer, max_local_moving_iterations, )?; - clustering.merge_clustering(&induced_network_clustering); } Ok(improved) } @@ -557,228 +509,6 @@ mod tests { }) } - #[test] - fn test_max_outer_iterations_zero_preserves_default_behavior() { - use rand::SeedableRng; - use rand::rngs::SmallRng; - - let edges = edge_list(); - let mut builder: LabeledNetworkBuilder = LabeledNetworkBuilder::new(); - let labeled_network: LabeledNetwork = builder.build(edges.into_iter(), true); - - let mut rng1: SmallRng = SmallRng::seed_from_u64(123); - let mut rng2: SmallRng = SmallRng::seed_from_u64(123); - - // max_outer_iterations = None (0 internally) should be identical to not limiting - let (_, clustering_none) = leiden( - labeled_network.compact(), - None, - Some(1), - None, - None, - &mut rng1, - true, - None, - None, - ) - .unwrap(); - - let (_, clustering_zero) = leiden( - labeled_network.compact(), - None, - Some(1), - None, - None, - &mut rng2, - true, - Some(0), - None, - ) - .unwrap(); - - for node_id in 0..labeled_network.num_nodes() { - assert_eq!( - clustering_none.cluster_at(node_id).unwrap(), - clustering_zero.cluster_at(node_id).unwrap(), - "Node {} differed between None and Some(0) for max_outer_iterations", - node_id - ); - } - } - - #[test] - fn test_max_outer_iterations_one_prevents_recursion() { - use rand::SeedableRng; - use rand::rngs::SmallRng; - - // Build a network with enough structure that Leiden would normally recurse - // (aggregate and re-run). With max_outer_iterations=1, recursion is blocked. - let edges: Vec = vec![ - ("a".into(), "b".into(), 10.0), - ("b".into(), "c".into(), 10.0), - ("c".into(), "a".into(), 10.0), - ("d".into(), "e".into(), 10.0), - ("e".into(), "f".into(), 10.0), - ("f".into(), "d".into(), 10.0), - ("g".into(), "h".into(), 10.0), - ("h".into(), "i".into(), 10.0), - ("i".into(), "g".into(), 10.0), - ("a".into(), "d".into(), 1.0), - ("d".into(), "g".into(), 1.0), - ("g".into(), "a".into(), 1.0), - ]; - - let mut builder: LabeledNetworkBuilder = LabeledNetworkBuilder::new(); - let labeled_network: LabeledNetwork = builder.build(edges.into_iter(), true); - - let mut rng: SmallRng = SmallRng::seed_from_u64(42); - - let (_, clustering_limited) = leiden( - labeled_network.compact(), - None, - Some(1), - None, - None, - &mut rng, - true, - Some(1), - None, - ) - .unwrap(); - - // Should still produce a valid clustering - for node_id in 0..labeled_network.num_nodes() { - assert!(clustering_limited.cluster_at(node_id).is_ok()); - } - - // Compare with unlimited recursion - let mut rng2: SmallRng = SmallRng::seed_from_u64(42); - let (_, clustering_unlimited) = leiden( - labeled_network.compact(), - None, - Some(1), - None, - None, - &mut rng2, - true, - None, - None, - ) - .unwrap(); - - // Both should produce valid clusterings. The exact number of clusters is not guaranteed to be - // monotonic w.r.t. recursion depth, so only sanity-check bounds here. - let limited_clusters = clustering_limited.next_cluster_id(); - let unlimited_clusters = clustering_unlimited.next_cluster_id(); - assert!(limited_clusters >= 1 && limited_clusters <= labeled_network.num_nodes()); - assert!(unlimited_clusters >= 1 && unlimited_clusters <= labeled_network.num_nodes()); - } - - #[test] - fn test_max_outer_iterations_higher_values_allow_more_recursion() { - use rand::SeedableRng; - use rand::rngs::SmallRng; - - // Build a deeper network that benefits from multiple recursion levels - let edges: Vec = vec![ - ("a".into(), "b".into(), 10.0), - ("b".into(), "c".into(), 10.0), - ("c".into(), "a".into(), 10.0), - ("d".into(), "e".into(), 10.0), - ("e".into(), "f".into(), 10.0), - ("f".into(), "d".into(), 10.0), - ("g".into(), "h".into(), 10.0), - ("h".into(), "i".into(), 10.0), - ("i".into(), "g".into(), 10.0), - ("j".into(), "k".into(), 10.0), - ("k".into(), "l".into(), 10.0), - ("l".into(), "j".into(), 10.0), - ("a".into(), "d".into(), 1.0), - ("d".into(), "g".into(), 1.0), - ("g".into(), "j".into(), 1.0), - ("j".into(), "a".into(), 1.0), - ]; - - let mut builder: LabeledNetworkBuilder = LabeledNetworkBuilder::new(); - let labeled_network: LabeledNetwork = builder.build(edges.into_iter(), true); - - let mut rng1: SmallRng = SmallRng::seed_from_u64(77); - let mut rng2: SmallRng = SmallRng::seed_from_u64(77); - - let (_, clustering_depth1) = leiden( - labeled_network.compact(), - None, - Some(1), - None, - None, - &mut rng1, - true, - Some(1), - None, - ) - .unwrap(); - - let (_, clustering_depth5) = leiden( - labeled_network.compact(), - None, - Some(1), - None, - None, - &mut rng2, - true, - Some(5), - None, - ) - .unwrap(); - - // The number of clusters is not guaranteed to be monotonic w.r.t. recursion depth; just - // sanity-check that both runs produce a bounded, non-empty clustering. - let depth1_clusters = clustering_depth1.next_cluster_id(); - let depth5_clusters = clustering_depth5.next_cluster_id(); - assert!(depth1_clusters >= 1 && depth1_clusters <= labeled_network.num_nodes()); - assert!(depth5_clusters >= 1 && depth5_clusters <= labeled_network.num_nodes()); - } - - #[test] - fn test_both_limits_together_produce_valid_clustering() { - use rand::SeedableRng; - use rand::rngs::SmallRng; - - let edges = edge_list(); - let mut builder: LabeledNetworkBuilder = LabeledNetworkBuilder::new(); - let labeled_network: LabeledNetwork = builder.build(edges.into_iter(), true); - - let mut rng: SmallRng = SmallRng::seed_from_u64(55); - - // Apply both limits simultaneously - let (improved, clustering) = leiden( - labeled_network.compact(), - None, - Some(1), - None, - None, - &mut rng, - true, - Some(2), - Some(2), - ) - .unwrap(); - - // Should have produced some result (improved or not is fine with limits) - let _ = improved; - - // Every node should have a valid cluster - for node_id in 0..labeled_network.num_nodes() { - assert!(clustering.cluster_at(node_id).is_ok()); - } - - // Should have at least 1 cluster - assert!( - clustering.next_cluster_id() >= 1, - "Should have at least 1 cluster" - ); - } - #[test] fn test_max_local_moving_iterations_through_leiden() { use rand::SeedableRng; @@ -800,7 +530,6 @@ mod tests { None, &mut rng1, true, - None, Some(1), ) .unwrap(); @@ -814,7 +543,6 @@ mod tests { None, &mut rng2, true, - None, Some(0), ) .unwrap(); diff --git a/packages/network_partitions/src/network/csr_view.rs b/packages/network_partitions/src/network/csr_view.rs index 93a491e..f021efb 100644 --- a/packages/network_partitions/src/network/csr_view.rs +++ b/packages/network_partitions/src/network/csr_view.rs @@ -456,18 +456,8 @@ mod tests { let compact = view.to_compact_network(); let mut rng = SmallRng::seed_from_u64(42); - let (improved, clustering) = leiden( - &compact, - None, - Some(1), - None, - None, - &mut rng, - true, - None, - None, - ) - .unwrap(); + let (improved, clustering) = + leiden(&compact, None, Some(1), None, None, &mut rng, true, None).unwrap(); assert!(improved); // Should find 2 communities diff --git a/packages/pyo3/src/lib.rs b/packages/pyo3/src/lib.rs index 97f154e..58876b9 100644 --- a/packages/pyo3/src/lib.rs +++ b/packages/pyo3/src/lib.rs @@ -1,24 +1,17 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -#![allow(unused_imports)] - mod errors; mod mediator; mod scipy_csr; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use numpy::PyReadonlyArray1; use pyo3::PyTypeInfo; use pyo3::prelude::*; -use network_partitions::clustering::Clustering; -use network_partitions::errors::CoreError; use network_partitions::network::prelude::*; -use network_partitions::quality; - -use network_partitions::safe_vectors::SafeVectors; use errors::*; @@ -70,29 +63,26 @@ impl HierarchicalCluster { /// :param float randomness: Default is `0.001`. The larger the randomness value, the more /// exploration of the partition space is possible. This is a major difference from the Louvain /// algorithm. The Louvain algorithm is purely greedy in the partition exploration. -/// :param int iterations: Default is `1`. The leiden algorithm is recursive, but subject to pseudo-random -/// number generators which sometimes lead to suboptimal community membership. Setting a number -/// greater than 1 will force leiden to run at minimum `iterations - 1` more times seeking a -/// more optimal partitioning. +/// :param int iterations: Default is `1`. How many times to run the full Leiden cycle +/// (local moving → refinement → recursive aggregation) on the original network. Each +/// iteration uses the previous result as its starting clustering, giving the algorithm +/// additional chances to escape suboptimal partitions. This is distinct from ``trials``, +/// which runs independent attempts and keeps the best result. /// :param bool use_modularity: Default is `True`. Whether to use modularity or CPM as the /// maximization function. /// :param Optional[int] seed: Default is `None`. If provided, the seed will be used in creating the /// Pseudo-Random Number Generator at a known state, making runs over the same network and /// starting_communities with the same parameters end with the same results. -/// :param int trials: Default is `1`. Leiden will be run repeatedly, keeping the best clustering -/// as per the maximization function. At the end of `repetitions` it will return the best -/// clustering. -/// :param Optional[int] max_outer_iterations: Default is `None`. When set, limits the recursion -/// depth of the Leiden algorithm's aggregation phase. A value of 1 means no recursive -/// aggregation is performed (only local moving + refinement on the original network). When -/// `None` or 0, the algorithm recurses until convergence (default behavior). +/// :param int trials: Default is `1`. Number of independent Leiden runs. Each trial starts +/// from scratch (or from ``starting_communities`` if provided) and the result with the +/// highest quality score is returned. /// :param Optional[int] max_local_moving_iterations: Default is `None`. When set, limits the /// number of sweeps through the local-moving work queue. One sweep is defined as processing -/// `N` node-pop operations from the queue, where `N` is the number of nodes in the network. -/// A value of 1 therefore caps local moving at `N` queue pops for that phase. When `None` or 0, -/// local moving continues until convergence (default behavior). -/// :return: The modularity of the best community partitioning and a dictionary of node to community -/// ids. The community ids will start at 0 and increment. +/// ``N`` node-pop operations from the queue, where ``N`` is the number of nodes in the +/// network. A value of 1 therefore caps local moving at ``N`` queue pops for that phase. +/// When ``None`` or 0, local moving continues until convergence (default behavior). +/// :return: The quality score of the best community partitioning and a dictionary of node to +/// community ids. The community ids will start at 0 and increment. /// :rtype: Tuple[float, Dict[str, int]] /// :raises ClusterIndexingError: /// :raises EmptyNetworkError: @@ -100,7 +90,7 @@ impl HierarchicalCluster { /// :raises ParameterRangeError: One of the parameters provided did not meet the requirements in the documentation. /// :raises UnsafeInducementError: An internal algorithm error. Please report with reproduction steps. #[pyfunction] -#[pyo3(signature=(/, edges, starting_communities=None, resolution=1.0, randomness=0.001, iterations=1, use_modularity=true, seed=None, trials=1, max_outer_iterations=None, max_local_moving_iterations=None))] +#[pyo3(signature=(/, edges, starting_communities=None, resolution=1.0, randomness=0.001, iterations=1, use_modularity=true, seed=None, trials=1, max_local_moving_iterations=None))] fn leiden( py: Python, edges: Vec, @@ -111,7 +101,6 @@ fn leiden( use_modularity: bool, seed: Option, trials: u64, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> PyResult<(f64, HashMap)> { let result: Result<(f64, HashMap), PyLeidenError> = py.detach(move || { @@ -124,7 +113,6 @@ fn leiden( use_modularity, seed, trials, - max_outer_iterations, max_local_moving_iterations, ) }); @@ -160,10 +148,10 @@ fn leiden( /// :param float randomness: Default is `0.001`. The larger the randomness value, the more /// exploration of the partition space is possible. This is a major difference from the Louvain /// algorithm. The Louvain algorithm is purely greedy in the partition exploration. -/// :param int iterations: Default is `1`. The leiden algorithm is recursive, but subject to pseudo-random -/// number generators which sometimes lead to suboptimal community membership. Setting a number -/// greater than 1 will force leiden to run at minimum `iterations - 1` more times seeking a -/// more optimal partitioning. +/// :param int iterations: Default is `1`. How many times to run the full Leiden cycle +/// (local moving → refinement → recursive aggregation) on the original network. Each +/// iteration uses the previous result as its starting clustering, giving the algorithm +/// additional chances to escape suboptimal partitions. /// :param bool use_modularity: Default is `True`. Whether to use modularity or CPM as the /// maximization function. /// :param int max_cluster_size: Default is `1000`. Any cluster larger than 1000 will be broken into @@ -171,10 +159,6 @@ fn leiden( /// :param Optional[int] seed: Default is `None`. If provided, the seed will be used in creating the /// Pseudo-Random Number Generator at a known state, making runs over the same network and /// starting_communities with the same parameters end with the same results. -/// :param Optional[int] max_outer_iterations: Default is `None`. When set, limits the recursion -/// depth of the Leiden algorithm's aggregation phase. A value of 1 means no recursive -/// aggregation is performed (only local moving + refinement on the original network). When -/// `None` or 0, the algorithm recurses until convergence (default behavior). /// :param Optional[int] max_local_moving_iterations: Default is `None`. When set, limits the /// number of sweeps through the local-moving work queue. One sweep is defined as processing /// `N` node-pop operations from the queue, where `N` is the number of nodes in the network. @@ -190,7 +174,7 @@ fn leiden( /// :raises ParameterRangeError: One of the parameters provided did not meet the requirements in the documentation. /// :raises UnsafeInducementError: An internal algorithm error. Please report with reproduction steps. #[pyfunction] -#[pyo3(signature=(/, edges, starting_communities=None, resolution=1.0, randomness=0.001, iterations=1, use_modularity=true, max_cluster_size=1000, seed=None, max_outer_iterations=None, max_local_moving_iterations=None))] +#[pyo3(signature=(/, edges, starting_communities=None, resolution=1.0, randomness=0.001, iterations=1, use_modularity=true, max_cluster_size=1000, seed=None, max_local_moving_iterations=None))] fn hierarchical_leiden( py: Python, edges: Vec, @@ -201,7 +185,6 @@ fn hierarchical_leiden( use_modularity: bool, max_cluster_size: u32, seed: Option, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> PyResult> { let result: Result, PyLeidenError> = py.detach(move || { @@ -214,7 +197,6 @@ fn hierarchical_leiden( use_modularity, max_cluster_size, seed, - max_outer_iterations, max_local_moving_iterations, ) }); @@ -274,17 +256,19 @@ fn modularity( /// and lower resolution values leads to fewer communities. Must be greater than 0. /// :param float randomness: Default is `0.001`. The larger the randomness value, the more /// exploration of the partition space is possible. -/// :param int iterations: Default is `1`. Number of times to run the full Leiden algorithm. +/// :param int iterations: Default is `1`. How many times to run the full Leiden cycle on the +/// original network. Each iteration uses the previous clustering as its starting point. /// :param bool use_modularity: Default is `True`. Whether to use modularity or CPM. /// :param Optional[int] seed: Default is `None`. Random seed for reproducibility. /// :param int trials: Default is `1`. Number of independent runs, returning the best result. -/// :param Optional[int] max_outer_iterations: Default is `None`. Limits recursion depth. -/// :param Optional[int] max_local_moving_iterations: Default is `None`. Limits local moving sweeps. +/// :param Optional[int] max_local_moving_iterations: Default is `None`. When set, limits the +/// number of sweeps through the local-moving work queue. When ``None`` or 0, local moving +/// continues until convergence. /// :return: The quality score and a dictionary mapping node ID (int) to community ID (int). /// :rtype: Tuple[float, Dict[int, int]] /// :raises ParameterRangeError: If CSR validation fails or parameters are out of range. #[pyfunction] -#[pyo3(signature=(/, indptr, indices, data, n_nodes, resolution=1.0, randomness=0.001, iterations=1, use_modularity=true, seed=None, trials=1, max_outer_iterations=None, max_local_moving_iterations=None))] +#[pyo3(signature=(/, indptr, indices, data, n_nodes, resolution=1.0, randomness=0.001, iterations=1, use_modularity=true, seed=None, trials=1, max_local_moving_iterations=None))] fn leiden_csr<'py>( py: Python<'py>, indptr: PyReadonlyArray1<'py, i64>, @@ -297,7 +281,6 @@ fn leiden_csr<'py>( use_modularity: bool, seed: Option, trials: u64, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> PyResult<(f64, HashMap)> { let indptr_slice = indptr.as_slice()?; @@ -325,7 +308,6 @@ fn leiden_csr<'py>( use_modularity, seed, trials, - max_outer_iterations, max_local_moving_iterations, ) }); diff --git a/packages/pyo3/src/mediator.rs b/packages/pyo3/src/mediator.rs index 4ab253b..d1746de 100644 --- a/packages/pyo3/src/mediator.rs +++ b/packages/pyo3/src/mediator.rs @@ -1,20 +1,18 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use network_partitions::clustering::Clustering; use network_partitions::errors::CoreError; use network_partitions::leiden; use network_partitions::network::prelude::*; use network_partitions::quality; -use network_partitions::safe_vectors::SafeVectors; use super::HierarchicalCluster; use super::errors::PyLeidenError; -use crate::errors::InvalidCommunityMappingError; +use rand::SeedableRng; use rand::rngs::SmallRng; -use rand::{Rng, RngExt, SeedableRng}; pub fn leiden( edges: Vec, @@ -25,9 +23,14 @@ pub fn leiden( use_modularity: bool, seed: Option, trials: u64, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> Result<(f64, HashMap), PyLeidenError> { + if trials == 0 { + return Err(PyLeidenError::ParameterRangeError( + "trials must be >= 1".to_string(), + )); + } + let mut builder: LabeledNetworkBuilder = LabeledNetworkBuilder::new(); let labeled_network: LabeledNetwork = builder.build(edges.into_iter(), use_modularity); @@ -58,7 +61,6 @@ pub fn leiden( Some(randomness), &mut rng, use_modularity, - max_outer_iterations, max_local_moving_iterations, )?; @@ -104,7 +106,6 @@ pub fn hierarchical_leiden( use_modularity: bool, max_cluster_size: u32, seed: Option, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> Result, PyLeidenError> { let mut builder: LabeledNetworkBuilder = LabeledNetworkBuilder::new(); @@ -132,7 +133,6 @@ pub fn hierarchical_leiden( &mut rng, use_modularity, max_cluster_size, - max_outer_iterations, max_local_moving_iterations, )?; @@ -175,7 +175,6 @@ pub fn leiden_csr( use_modularity: bool, seed: Option, trials: u64, - max_outer_iterations: Option, max_local_moving_iterations: Option, ) -> Result<(f64, HashMap), PyLeidenError> { use crate::scipy_csr::ScipyCsrView; @@ -211,7 +210,6 @@ pub fn leiden_csr( Some(randomness), &mut rng, use_modularity, - max_outer_iterations, max_local_moving_iterations, )?; diff --git a/packages/pyo3/tests/test_leiden.py b/packages/pyo3/tests/test_leiden.py index ea51831..5d1879d 100644 --- a/packages/pyo3/tests/test_leiden.py +++ b/packages/pyo3/tests/test_leiden.py @@ -110,7 +110,7 @@ def test_leiden_csr_basic(self): use_modularity=True, seed=42, trials=1, - max_outer_iterations=10, + max_local_moving_iterations=None, ) @@ -146,7 +146,7 @@ def test_leiden_csr_deterministic_with_seed(self): indptr=indptr, indices=indices, data=data, n_nodes=5, resolution=1.0, randomness=0.01, iterations=2, use_modularity=True, seed=99, trials=1, - max_outer_iterations=10, max_local_moving_iterations=None, + max_local_moving_iterations=None, ) _, p1 = gcn.leiden_csr(**kwargs) _, p2 = gcn.leiden_csr(**kwargs) @@ -175,7 +175,7 @@ def test_leiden_csr_trials_improves_or_equals(self): indptr=indptr, indices=indices, data=data, n_nodes=6, resolution=1.0, randomness=0.01, iterations=2, use_modularity=True, seed=42, - max_outer_iterations=10, max_local_moving_iterations=None, + max_local_moving_iterations=None, ) q1, _ = gcn.leiden_csr(trials=1, **kwargs) q5, _ = gcn.leiden_csr(trials=5, **kwargs)