Skip to content

Commit a5d74f9

Browse files
isPANNclaude
andcommitted
Make MIS → KingsSubgraph reduction deterministic (#1061)
Two compounding sources of non-determinism in the greedy path decomposition were making `pred reduce` vary ~15-20% in target-graph size across runs: - `greedy_step` used `rand::rng()` (unseeded thread-local RNG) for tie-breaking. - `AdjList` was `Vec<HashSet<usize>>`; per-process HashSet iteration order leaked into `layout.neighbors` via pushes in `vsep_updated_neighbors`, so even a fixed RNG seed would still drift. Fix: - Switch `AdjList` to `Vec<BTreeSet<usize>>` so iteration is sorted. - Thread a seeded `SmallRng` through `greedy_step` / `greedy_decompose` / `pathwidth`, using `DEFAULT_PATHWIDTH_SEED = 0` for the public entry point. - Expose `pathwidth_with_seed(.., seed)` as an escape hatch for callers that want to sample the layout space (e.g. future minimum-atom search in #1062). Three CLI invocations of `pred reduce` on a 64-vertex MIS now produce byte-identical bundles. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b3b18f1 commit a5d74f9

3 files changed

Lines changed: 167 additions & 12 deletions

File tree

src/rules/unitdiskmapping/pathdecomposition.rs

Lines changed: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,30 @@
1414
//! Experimental evaluation of a branch and bound algorithm for computing pathwidth.
1515
//! <https://doi.org/10.1007/978-3-319-07959-2_5>
1616
17+
use rand::rngs::SmallRng;
1718
use rand::seq::IndexedRandom;
18-
use std::collections::{HashMap, HashSet};
19+
use rand::SeedableRng;
20+
use std::collections::{BTreeSet, HashMap, HashSet};
21+
22+
/// Default seed used when no explicit seed is passed to [`pathwidth`].
23+
///
24+
/// Keeping a fixed default makes [`pathwidth`] and [`greedy_decompose`] deterministic
25+
/// across repeated invocations with the same input, which reductions and downstream
26+
/// benchmarks rely on for reproducibility. Callers that want diverse layouts can use
27+
/// [`pathwidth_with_seed`] instead.
28+
pub const DEFAULT_PATHWIDTH_SEED: u64 = 0;
1929

2030
/// Adjacency list representation built once from an edge list.
21-
type AdjList = Vec<HashSet<usize>>;
31+
///
32+
/// Uses `BTreeSet` rather than `HashSet` so iteration order is deterministic
33+
/// (sorted by vertex index). Several places in this module push from `adj[v]`
34+
/// into the layout's neighbor list; HashSet iteration order leaked into the
35+
/// vertex ordering and caused non-reproducible path decompositions.
36+
type AdjList = Vec<BTreeSet<usize>>;
2237

2338
/// Build an adjacency list from an edge list.
2439
fn build_adj(num_vertices: usize, edges: &[(usize, usize)]) -> AdjList {
25-
let mut adj: Vec<HashSet<usize>> = vec![HashSet::new(); num_vertices];
40+
let mut adj: AdjList = vec![BTreeSet::new(); num_vertices];
2641
for &(u, v) in edges {
2742
adj[u].insert(v);
2843
adj[v].insert(u);
@@ -258,8 +273,14 @@ fn greedy_exact(adj: &AdjList, mut layout: Layout) -> Layout {
258273

259274
/// Perform one greedy step by choosing the best vertex from a list.
260275
///
261-
/// Selects randomly among vertices that minimize the new vsep.
262-
fn greedy_step(adj: &AdjList, layout: &Layout, list: &[usize]) -> Layout {
276+
/// Selects among vertices that minimize the new vsep, breaking ties using the
277+
/// provided RNG. Passing an explicit RNG makes tie-breaking deterministic given a seed.
278+
fn greedy_step<R: rand::Rng + ?Sized>(
279+
adj: &AdjList,
280+
layout: &Layout,
281+
list: &[usize],
282+
rng: &mut R,
283+
) -> Layout {
263284
let layouts: Vec<Layout> = list.iter().map(|&v| extend(adj, layout, v)).collect();
264285

265286
let costs: Vec<usize> = layouts.iter().map(|l| l.vsep()).collect();
@@ -272,27 +293,43 @@ fn greedy_step(adj: &AdjList, layout: &Layout, list: &[usize]) -> Layout {
272293
.map(|(i, _)| i)
273294
.collect();
274295

275-
let mut rng = rand::rng();
276-
let &chosen_idx = best_indices.as_slice().choose(&mut rng).unwrap();
296+
let &chosen_idx = best_indices.as_slice().choose(rng).unwrap();
277297

278298
layouts.into_iter().nth(chosen_idx).unwrap()
279299
}
280300

281301
/// Compute a path decomposition using the greedy algorithm.
282302
///
283-
/// This combines exact rules (that don't increase pathwidth) with
284-
/// greedy choices when exact rules don't apply.
303+
/// Uses a fixed default seed (see [`DEFAULT_PATHWIDTH_SEED`]) so repeated calls on
304+
/// the same input produce the same layout. Use [`pathwidth_with_seed`] for variation.
305+
///
306+
/// Only exposed for unit tests; production code reaches the greedy path through
307+
/// [`pathwidth`] or [`pathwidth_with_seed`].
308+
#[cfg(test)]
285309
pub fn greedy_decompose(num_vertices: usize, edges: &[(usize, usize)]) -> Layout {
310+
let mut rng = SmallRng::seed_from_u64(DEFAULT_PATHWIDTH_SEED);
311+
greedy_decompose_with_rng(num_vertices, edges, &mut rng)
312+
}
313+
314+
/// Compute a path decomposition using the greedy algorithm with a caller-supplied RNG.
315+
///
316+
/// This combines exact rules (that don't increase pathwidth) with greedy choices
317+
/// when exact rules don't apply. Random tie-breaking draws from `rng`.
318+
fn greedy_decompose_with_rng<R: rand::Rng + ?Sized>(
319+
num_vertices: usize,
320+
edges: &[(usize, usize)],
321+
rng: &mut R,
322+
) -> Layout {
286323
let adj = build_adj(num_vertices, edges);
287324
let mut layout = Layout::empty(num_vertices);
288325

289326
loop {
290327
layout = greedy_exact(&adj, layout);
291328

292329
if !layout.neighbors.is_empty() {
293-
layout = greedy_step(&adj, &layout, &layout.neighbors.clone());
330+
layout = greedy_step(&adj, &layout, &layout.neighbors.clone(), rng);
294331
} else if !layout.disconnected.is_empty() {
295-
layout = greedy_step(&adj, &layout, &layout.disconnected.clone());
332+
layout = greedy_step(&adj, &layout, &layout.disconnected.clone(), rng);
296333
} else {
297334
break;
298335
}
@@ -423,6 +460,23 @@ pub fn pathwidth(
423460
num_vertices: usize,
424461
edges: &[(usize, usize)],
425462
method: PathDecompositionMethod,
463+
) -> Layout {
464+
pathwidth_with_seed(num_vertices, edges, method, DEFAULT_PATHWIDTH_SEED)
465+
}
466+
467+
/// Like [`pathwidth`], but with a caller-chosen RNG seed for greedy tie-breaking.
468+
///
469+
/// The greedy path-decomposition algorithm uses random choices to break ties
470+
/// between candidate vertices with the same vertex separation. A single `SmallRng`
471+
/// seeded with `seed` is threaded through all restarts, so restarts remain diverse
472+
/// (each advances the RNG state) while the overall output is reproducible.
473+
///
474+
/// `MinhThiTrick` and `Auto` on small graphs are deterministic and ignore the seed.
475+
pub fn pathwidth_with_seed(
476+
num_vertices: usize,
477+
edges: &[(usize, usize)],
478+
method: PathDecompositionMethod,
479+
seed: u64,
426480
) -> Layout {
427481
let method = match method {
428482
PathDecompositionMethod::Auto => {
@@ -438,9 +492,10 @@ pub fn pathwidth(
438492
PathDecompositionMethod::Greedy { nrepeat } => {
439493
// Defend against direct enum construction with nrepeat = 0.
440494
let nrepeat = nrepeat.max(1);
495+
let mut rng = SmallRng::seed_from_u64(seed);
441496
let mut best: Option<Layout> = None;
442497
for _ in 0..nrepeat {
443-
let layout = greedy_decompose(num_vertices, edges);
498+
let layout = greedy_decompose_with_rng(num_vertices, edges, &mut rng);
444499
if best.is_none() || layout.vsep() < best.as_ref().unwrap().vsep() {
445500
best = Some(layout);
446501
}

src/unit_tests/rules/maximumindependentset_gridgraph.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,46 @@ fn test_map_unweighted_produces_uniform_weights() {
3232
);
3333
}
3434

35+
#[test]
36+
fn test_mis_simple_one_to_kings_one_is_deterministic_on_large_graph() {
37+
// Regression for #1061: `pred reduce` output was non-deterministic because the
38+
// greedy path decomposition used an unseeded thread RNG for tie-breaking and
39+
// the adjacency list used HashSet iteration order. A 64-vertex graph matches
40+
// the reporter's scenario (10x10 kings, p=0.3) and forces the Auto path to
41+
// pick the Greedy branch (>30 vertices).
42+
let n = 64;
43+
let mut edges: Vec<(usize, usize)> = Vec::new();
44+
for r in 0..8 {
45+
for c in 0..8 {
46+
let v = r * 8 + c;
47+
if c + 1 < 8 {
48+
edges.push((v, r * 8 + c + 1));
49+
}
50+
if r + 1 < 8 {
51+
edges.push((v, (r + 1) * 8 + c));
52+
}
53+
if r + 1 < 8 && c + 1 < 8 {
54+
edges.push((v, (r + 1) * 8 + c + 1));
55+
}
56+
}
57+
}
58+
59+
let problem = MaximumIndependentSet::new(SimpleGraph::new(n, edges), vec![One; n]);
60+
61+
let first = ReduceTo::<MaximumIndependentSet<KingsSubgraph, One>>::reduce_to(&problem);
62+
let baseline_atoms = first.target_problem().graph().num_vertices();
63+
let baseline_edges = first.target_problem().graph().edges().len();
64+
65+
for _ in 0..3 {
66+
let again = ReduceTo::<MaximumIndependentSet<KingsSubgraph, One>>::reduce_to(&problem);
67+
assert_eq!(
68+
again.target_problem().graph().num_vertices(),
69+
baseline_atoms,
70+
);
71+
assert_eq!(again.target_problem().graph().edges().len(), baseline_edges);
72+
}
73+
}
74+
3575
#[test]
3676
fn test_mis_simple_one_to_kings_one_closed_loop() {
3777
// Path graph: 0-1-2-3-4 (MIS = 3: select vertices 0, 2, 4)

src/unit_tests/rules/unitdiskmapping/pathdecomposition.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,66 @@ fn test_pathwidth_auto_large() {
219219
assert_eq!(layout.vsep(), 1); // Path graph has pathwidth 1
220220
}
221221

222+
#[test]
223+
fn test_pathwidth_greedy_is_deterministic() {
224+
// Reproduces issue #1061: pathwidth greedy must be deterministic across calls.
225+
// Graph is large enough that Auto picks Greedy and tie-breaking in greedy_step
226+
// is exercised (grid with many symmetric choices).
227+
let n = 64;
228+
let mut edges: Vec<(usize, usize)> = Vec::new();
229+
for r in 0..8 {
230+
for c in 0..8 {
231+
let v = r * 8 + c;
232+
if c + 1 < 8 {
233+
edges.push((v, r * 8 + c + 1));
234+
}
235+
if r + 1 < 8 {
236+
edges.push((v, (r + 1) * 8 + c));
237+
}
238+
}
239+
}
240+
241+
let first = pathwidth(n, &edges, PathDecompositionMethod::Auto);
242+
for _ in 0..3 {
243+
let other = pathwidth(n, &edges, PathDecompositionMethod::Auto);
244+
assert_eq!(other.vertices, first.vertices);
245+
assert_eq!(other.vsep(), first.vsep());
246+
}
247+
}
248+
249+
#[test]
250+
fn test_pathwidth_with_seed_varies_output() {
251+
// Different seeds may produce different layouts (same pathwidth or better/worse).
252+
// This verifies the seed API actually affects tie-breaking.
253+
let n = 64;
254+
let mut edges: Vec<(usize, usize)> = Vec::new();
255+
for r in 0..8 {
256+
for c in 0..8 {
257+
let v = r * 8 + c;
258+
if c + 1 < 8 {
259+
edges.push((v, r * 8 + c + 1));
260+
}
261+
if r + 1 < 8 {
262+
edges.push((v, (r + 1) * 8 + c));
263+
}
264+
}
265+
}
266+
267+
let method = PathDecompositionMethod::greedy();
268+
let a = pathwidth_with_seed(n, &edges, method, 0);
269+
let b = pathwidth_with_seed(n, &edges, method, 0);
270+
assert_eq!(a.vertices, b.vertices, "same seed → same layout");
271+
272+
// At least one of the sampled seeds should produce a different ordering,
273+
// confirming the seed actually threads into tie-breaking.
274+
let differs =
275+
(1u64..=10).any(|s| pathwidth_with_seed(n, &edges, method, s).vertices != a.vertices);
276+
assert!(
277+
differs,
278+
"expected some seed in 1..=10 to produce a different vertex ordering than seed 0",
279+
);
280+
}
281+
222282
// === Ground truth tests from JSON dataset ===
223283

224284
/// Compute vsep from scratch for a given vertex ordering on a graph.

0 commit comments

Comments
 (0)