Skip to content

Commit 7155f6a

Browse files
daxpryceCopilotCopilot
authored
feat: add max_outer_iterations and max_local_moving_iterations params (#68)
* feat: add max_outer_iterations and max_local_moving_iterations parameters Add optional loop control parameters to leiden and hierarchical_leiden: - max_outer_iterations: limits recursion depth in the Leiden cycle (local-move → refine → aggregate). 0/None = unlimited (current behavior). - max_local_moving_iterations: limits sweeps in the local moving phase. One sweep = processing N nodes from the work queue. 0/None = unlimited. These enable benchmarking partial-convergence modes without changing default behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address PR review feedback for loop control params - Use saturating_mul for max_nodes_to_process to prevent overflow on large graphs - Add Python docstrings for max_outer_iterations and max_local_moving_iterations on both leiden() and hierarchical_leiden() - Add unit tests for max_local_moving_iterations: - Verifies sweep limiting produces valid clustering - Verifies 0 means unlimited (matches u32::MAX behavior) - Verifies saturating_mul doesn't panic with large values - Add unit tests for max_outer_iterations: - Verifies 0/None preserves default unlimited behavior - Verifies 1 prevents recursion (flat clustering only) - Verifies higher values allow deeper aggregation - Verifies both limits together produce valid results - Verifies max_local_moving_iterations flows through leiden() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: apply cargo fmt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Removing uv.lock. Used for development. Do not want dependabot alerts because of uv's resolution. * Removing uv.lock. Used for development. Do not want dependabot alerts because of uv's resolution. * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent ed64993 commit 7155f6a

8 files changed

Lines changed: 501 additions & 258 deletions

File tree

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@ Cargo.lock
1313

1414
venv/
1515
__pycache__
16-
16+
uv.lock

packages/cli/src/leiden.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ pub fn leiden(
6060
Some(randomness),
6161
&mut rng,
6262
use_modularity,
63+
None,
64+
None,
6365
);
6466

6567
let leiden_completion_instant: Instant = Instant::now();

packages/network_partitions/src/leiden/full_network_clustering.rs

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub fn full_network_clustering<T>(
1414
clustering: &mut Clustering,
1515
adjusted_resolution: f64,
1616
rng: &mut T,
17+
max_local_moving_iterations: u32,
1718
) -> Result<bool, CoreError>
1819
where
1920
T: Rng,
@@ -36,7 +37,19 @@ where
3637
let mut neighboring_clusters: NeighboringClusters =
3738
NeighboringClusters::with_capacity(network.num_nodes());
3839

40+
let max_nodes_to_process: usize = if max_local_moving_iterations == 0 {
41+
usize::MAX
42+
} else {
43+
(max_local_moving_iterations as usize).saturating_mul(network.num_nodes())
44+
};
45+
let mut nodes_processed: usize = 0;
46+
3947
while !work_queue.is_empty() {
48+
if nodes_processed >= max_nodes_to_process {
49+
break;
50+
}
51+
nodes_processed += 1;
52+
4053
let current_node: usize = work_queue.pop_front()?;
4154
let current_cluster: usize = clustering.cluster_at(current_node)?;
4255
let current_node_weight: f64 = network.node_weight(current_node);
@@ -293,6 +306,7 @@ mod tests {
293306
&mut clustering,
294307
adjusted_resolution,
295308
&mut rng,
309+
0,
296310
)
297311
.unwrap();
298312

@@ -350,4 +364,150 @@ mod tests {
350364
jarkko_cluster, nodes_per_cluster[jarkko_cluster]
351365
);
352366
}
367+
368+
#[test]
369+
fn test_max_local_moving_iterations_limits_sweeps() {
370+
let mut rng: SmallRng = SmallRng::seed_from_u64(42);
371+
372+
let edges: Vec<Edge> = vec![
373+
("a".into(), "b".into(), 10.0),
374+
("b".into(), "c".into(), 10.0),
375+
("c".into(), "d".into(), 10.0),
376+
("d".into(), "e".into(), 10.0),
377+
("e".into(), "f".into(), 10.0),
378+
("f".into(), "g".into(), 10.0),
379+
("g".into(), "h".into(), 10.0),
380+
("a".into(), "c".into(), 5.0),
381+
("b".into(), "d".into(), 5.0),
382+
("e".into(), "g".into(), 5.0),
383+
("f".into(), "h".into(), 5.0),
384+
];
385+
386+
let mut builder: LabeledNetworkBuilder<String> = LabeledNetworkBuilder::new();
387+
let labeled_network: LabeledNetwork<String> = builder.build(edges.into_iter(), true);
388+
389+
let mut clustering_limited: Clustering =
390+
Clustering::as_self_clusters(labeled_network.num_nodes());
391+
392+
let adjusted_resolution: f64 =
393+
resolution::adjust_resolution(Option::None, labeled_network.compact(), true);
394+
395+
// Run with max_local_moving_iterations = 1 (only one sweep)
396+
let _improved_limited = full_network_clustering(
397+
labeled_network.compact(),
398+
&mut clustering_limited,
399+
adjusted_resolution,
400+
&mut rng,
401+
1,
402+
)
403+
.unwrap();
404+
405+
// Should still produce a valid clustering (every node has a cluster)
406+
for node_id in 0..labeled_network.num_nodes() {
407+
assert!(clustering_limited.cluster_at(node_id).is_ok());
408+
}
409+
410+
// Now run with unlimited iterations for comparison
411+
let mut rng2: SmallRng = SmallRng::seed_from_u64(42);
412+
let mut clustering_unlimited: Clustering =
413+
Clustering::as_self_clusters(labeled_network.num_nodes());
414+
415+
let _improved_unlimited = full_network_clustering(
416+
labeled_network.compact(),
417+
&mut clustering_unlimited,
418+
adjusted_resolution,
419+
&mut rng2,
420+
0,
421+
)
422+
.unwrap();
423+
// Cluster count is not guaranteed to be monotonic with additional local-moving steps; only
424+
// sanity-check that both results are bounded and non-empty.
425+
let limited_clusters = clustering_limited.next_cluster_id();
426+
let unlimited_clusters = clustering_unlimited.next_cluster_id();
427+
assert!(limited_clusters >= 1 && limited_clusters <= labeled_network.num_nodes());
428+
assert!(unlimited_clusters >= 1 && unlimited_clusters <= labeled_network.num_nodes());
429+
}
430+
431+
#[test]
432+
fn test_max_local_moving_iterations_zero_means_unlimited() {
433+
let mut rng1: SmallRng = SmallRng::seed_from_u64(99);
434+
let mut rng2: SmallRng = SmallRng::seed_from_u64(99);
435+
436+
let edges: Vec<Edge> = vec![
437+
("a".into(), "b".into(), 10.0),
438+
("b".into(), "c".into(), 10.0),
439+
("c".into(), "a".into(), 10.0),
440+
("d".into(), "e".into(), 10.0),
441+
("e".into(), "f".into(), 10.0),
442+
("f".into(), "d".into(), 10.0),
443+
("a".into(), "d".into(), 1.0),
444+
];
445+
446+
let mut builder: LabeledNetworkBuilder<String> = LabeledNetworkBuilder::new();
447+
let labeled_network: LabeledNetwork<String> = builder.build(edges.into_iter(), true);
448+
449+
let adjusted_resolution: f64 =
450+
resolution::adjust_resolution(Option::None, labeled_network.compact(), true);
451+
452+
let mut clustering1: Clustering = Clustering::as_self_clusters(labeled_network.num_nodes());
453+
let mut clustering2: Clustering = Clustering::as_self_clusters(labeled_network.num_nodes());
454+
455+
// max_local_moving_iterations = 0 should behave the same as no limit
456+
full_network_clustering(
457+
labeled_network.compact(),
458+
&mut clustering1,
459+
adjusted_resolution,
460+
&mut rng1,
461+
0,
462+
)
463+
.unwrap();
464+
465+
// Use a very large value that effectively means no limit
466+
full_network_clustering(
467+
labeled_network.compact(),
468+
&mut clustering2,
469+
adjusted_resolution,
470+
&mut rng2,
471+
u32::MAX,
472+
)
473+
.unwrap();
474+
475+
// Both should produce identical results
476+
for node_id in 0..labeled_network.num_nodes() {
477+
assert_eq!(
478+
clustering1.cluster_at(node_id).unwrap(),
479+
clustering2.cluster_at(node_id).unwrap(),
480+
"Node {} differed between 0 (unlimited) and u32::MAX",
481+
node_id
482+
);
483+
}
484+
}
485+
486+
#[test]
487+
fn test_max_local_moving_saturating_mul_no_panic() {
488+
// Verify that the saturating_mul doesn't panic even with large iteration counts
489+
// on a small network (would overflow if using regular multiplication on a large network)
490+
let mut rng: SmallRng = SmallRng::seed_from_u64(7);
491+
492+
let edges: Vec<Edge> = vec![("a".into(), "b".into(), 1.0), ("b".into(), "c".into(), 1.0)];
493+
494+
let mut builder: LabeledNetworkBuilder<String> = LabeledNetworkBuilder::new();
495+
let labeled_network: LabeledNetwork<String> = builder.build(edges.into_iter(), true);
496+
497+
let mut clustering: Clustering = Clustering::as_self_clusters(labeled_network.num_nodes());
498+
499+
let adjusted_resolution: f64 =
500+
resolution::adjust_resolution(Option::None, labeled_network.compact(), true);
501+
502+
// u32::MAX * num_nodes would overflow usize on 32-bit, but saturating_mul handles it
503+
let result = full_network_clustering(
504+
labeled_network.compact(),
505+
&mut clustering,
506+
adjusted_resolution,
507+
&mut rng,
508+
u32::MAX,
509+
);
510+
511+
assert!(result.is_ok());
512+
}
353513
}

packages/network_partitions/src/leiden/hierarchical.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ pub fn hierarchical_leiden<T>(
125125
rng: &mut T,
126126
use_modularity: bool,
127127
max_cluster_size: u32,
128+
max_outer_iterations: Option<u32>,
129+
max_local_moving_iterations: Option<u32>,
128130
) -> Result<Vec<HierarchicalCluster>, CoreError>
129131
where
130132
T: Rng + Clone + Send,
@@ -137,6 +139,8 @@ where
137139
randomness,
138140
rng,
139141
use_modularity,
142+
max_outer_iterations,
143+
max_local_moving_iterations,
140144
)?;
141145

142146
let mut hierarchical_clustering: HierarchicalClustering =
@@ -170,6 +174,8 @@ where
170174
randomness,
171175
rng,
172176
use_modularity,
177+
max_outer_iterations,
178+
max_local_moving_iterations,
173179
)?;
174180
let offset: usize = updated_clustering.next_cluster_id();
175181

0 commit comments

Comments
 (0)