Make MIS → KingsSubgraph reduction deterministic (#1061) (#1063)

* 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>

* Add triangular determinism regression test (#1061)

The triangular mapping shares the same `pathwidth` call as KSG, so it had
the same non-determinism before this PR. Pin the behavior with an
equivalent regression test so future changes to pathdecomposition can't
silently regress this mapping either.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: isPANN

src/rules/unitdiskmapping/pathdecomposition.rs

@@ -14,15 +14,30 @@
 //! Experimental evaluation of a branch and bound algorithm for computing pathwidth.
 //! <https://doi.org/10.1007/978-3-319-07959-2_5>
 
+use rand::rngs::SmallRng;
 use rand::seq::IndexedRandom;
-use std::collections::{HashMap, HashSet};
+use rand::SeedableRng;
+use std::collections::{BTreeSet, HashMap, HashSet};
+
+/// Default seed used when no explicit seed is passed to [`pathwidth`].
+///
+/// Keeping a fixed default makes [`pathwidth`] and [`greedy_decompose`] deterministic
+/// across repeated invocations with the same input, which reductions and downstream
+/// benchmarks rely on for reproducibility. Callers that want diverse layouts can use
+/// [`pathwidth_with_seed`] instead.
+pub const DEFAULT_PATHWIDTH_SEED: u64 = 0;
 
 /// Adjacency list representation built once from an edge list.
-type AdjList = Vec<HashSet<usize>>;
+///
+/// Uses `BTreeSet` rather than `HashSet` so iteration order is deterministic
+/// (sorted by vertex index). Several places in this module push from `adj[v]`
+/// into the layout's neighbor list; HashSet iteration order leaked into the
+/// vertex ordering and caused non-reproducible path decompositions.
+type AdjList = Vec<BTreeSet<usize>>;
 
 /// Build an adjacency list from an edge list.
 fn build_adj(num_vertices: usize, edges: &[(usize, usize)]) -> AdjList {
-    let mut adj: Vec<HashSet<usize>> = vec![HashSet::new(); num_vertices];
+    let mut adj: AdjList = vec![BTreeSet::new(); num_vertices];
     for &(u, v) in edges {
         adj[u].insert(v);
         adj[v].insert(u);
@@ -258,8 +273,14 @@ fn greedy_exact(adj: &AdjList, mut layout: Layout) -> Layout {
 
 /// Perform one greedy step by choosing the best vertex from a list.
 ///
-/// Selects randomly among vertices that minimize the new vsep.
-fn greedy_step(adj: &AdjList, layout: &Layout, list: &[usize]) -> Layout {
+/// Selects among vertices that minimize the new vsep, breaking ties using the
+/// provided RNG. Passing an explicit RNG makes tie-breaking deterministic given a seed.
+fn greedy_step<R: rand::Rng + ?Sized>(
+    adj: &AdjList,
+    layout: &Layout,
+    list: &[usize],
+    rng: &mut R,
+) -> Layout {
     let layouts: Vec<Layout> = list.iter().map(|&v| extend(adj, layout, v)).collect();
 
     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 {
         .map(|(i, _)| i)
         .collect();
 
-    let mut rng = rand::rng();
-    let &chosen_idx = best_indices.as_slice().choose(&mut rng).unwrap();
+    let &chosen_idx = best_indices.as_slice().choose(rng).unwrap();
 
     layouts.into_iter().nth(chosen_idx).unwrap()
 }
 
 /// Compute a path decomposition using the greedy algorithm.
 ///
-/// This combines exact rules (that don't increase pathwidth) with
-/// greedy choices when exact rules don't apply.
+/// Uses a fixed default seed (see [`DEFAULT_PATHWIDTH_SEED`]) so repeated calls on
+/// the same input produce the same layout. Use [`pathwidth_with_seed`] for variation.
+///
+/// Only exposed for unit tests; production code reaches the greedy path through
+/// [`pathwidth`] or [`pathwidth_with_seed`].
+#[cfg(test)]
 pub fn greedy_decompose(num_vertices: usize, edges: &[(usize, usize)]) -> Layout {
+    let mut rng = SmallRng::seed_from_u64(DEFAULT_PATHWIDTH_SEED);
+    greedy_decompose_with_rng(num_vertices, edges, &mut rng)
+}
+
+/// Compute a path decomposition using the greedy algorithm with a caller-supplied RNG.
+///
+/// This combines exact rules (that don't increase pathwidth) with greedy choices
+/// when exact rules don't apply. Random tie-breaking draws from `rng`.
+fn greedy_decompose_with_rng<R: rand::Rng + ?Sized>(
+    num_vertices: usize,
+    edges: &[(usize, usize)],
+    rng: &mut R,
+) -> Layout {
     let adj = build_adj(num_vertices, edges);
     let mut layout = Layout::empty(num_vertices);
 
     loop {
         layout = greedy_exact(&adj, layout);
 
         if !layout.neighbors.is_empty() {
-            layout = greedy_step(&adj, &layout, &layout.neighbors.clone());
+            layout = greedy_step(&adj, &layout, &layout.neighbors.clone(), rng);
         } else if !layout.disconnected.is_empty() {
-            layout = greedy_step(&adj, &layout, &layout.disconnected.clone());
+            layout = greedy_step(&adj, &layout, &layout.disconnected.clone(), rng);
         } else {
             break;
         }
@@ -423,6 +460,23 @@ pub fn pathwidth(
     num_vertices: usize,
     edges: &[(usize, usize)],
     method: PathDecompositionMethod,
+) -> Layout {
+    pathwidth_with_seed(num_vertices, edges, method, DEFAULT_PATHWIDTH_SEED)
+}
+
+/// Like [`pathwidth`], but with a caller-chosen RNG seed for greedy tie-breaking.
+///
+/// The greedy path-decomposition algorithm uses random choices to break ties
+/// between candidate vertices with the same vertex separation. A single `SmallRng`
+/// seeded with `seed` is threaded through all restarts, so restarts remain diverse
+/// (each advances the RNG state) while the overall output is reproducible.
+///
+/// `MinhThiTrick` and `Auto` on small graphs are deterministic and ignore the seed.
+pub fn pathwidth_with_seed(
+    num_vertices: usize,
+    edges: &[(usize, usize)],
+    method: PathDecompositionMethod,
+    seed: u64,
 ) -> Layout {
     let method = match method {
         PathDecompositionMethod::Auto => {
@@ -438,9 +492,10 @@ pub fn pathwidth(
         PathDecompositionMethod::Greedy { nrepeat } => {
             // Defend against direct enum construction with nrepeat = 0.
             let nrepeat = nrepeat.max(1);
+            let mut rng = SmallRng::seed_from_u64(seed);
             let mut best: Option<Layout> = None;
             for _ in 0..nrepeat {
-                let layout = greedy_decompose(num_vertices, edges);
+                let layout = greedy_decompose_with_rng(num_vertices, edges, &mut rng);
                 if best.is_none() || layout.vsep() < best.as_ref().unwrap().vsep() {
                     best = Some(layout);
                 }

src/unit_tests/rules/maximumindependentset_gridgraph.rs

@@ -32,6 +32,46 @@ fn test_map_unweighted_produces_uniform_weights() {
     );
 }
 
+#[test]
+fn test_mis_simple_one_to_kings_one_is_deterministic_on_large_graph() {
+    // Regression for #1061: `pred reduce` output was non-deterministic because the
+    // greedy path decomposition used an unseeded thread RNG for tie-breaking and
+    // the adjacency list used HashSet iteration order. A 64-vertex graph matches
+    // the reporter's scenario (10x10 kings, p=0.3) and forces the Auto path to
+    // pick the Greedy branch (>30 vertices).
+    let n = 64;
+    let mut edges: Vec<(usize, usize)> = Vec::new();
+    for r in 0..8 {
+        for c in 0..8 {
+            let v = r * 8 + c;
+            if c + 1 < 8 {
+                edges.push((v, r * 8 + c + 1));
+            }
+            if r + 1 < 8 {
+                edges.push((v, (r + 1) * 8 + c));
+            }
+            if r + 1 < 8 && c + 1 < 8 {
+                edges.push((v, (r + 1) * 8 + c + 1));
+            }
+        }
+    }
+
+    let problem = MaximumIndependentSet::new(SimpleGraph::new(n, edges), vec![One; n]);
+
+    let first = ReduceTo::<MaximumIndependentSet<KingsSubgraph, One>>::reduce_to(&problem);
+    let baseline_atoms = first.target_problem().graph().num_vertices();
+    let baseline_edges = first.target_problem().graph().edges().len();
+
+    for _ in 0..3 {
+        let again = ReduceTo::<MaximumIndependentSet<KingsSubgraph, One>>::reduce_to(&problem);
+        assert_eq!(
+            again.target_problem().graph().num_vertices(),
+            baseline_atoms,
+        );
+        assert_eq!(again.target_problem().graph().edges().len(), baseline_edges);
+    }
+}
+
 #[test]
 fn test_mis_simple_one_to_kings_one_closed_loop() {
     // Path graph: 0-1-2-3-4 (MIS = 3: select vertices 0, 2, 4)

src/unit_tests/rules/maximumindependentset_triangular.rs

@@ -3,6 +3,44 @@ use crate::models::graph::MaximumIndependentSet;
 use crate::topology::{Graph, SimpleGraph, TriangularSubgraph};
 use crate::types::One;
 
+#[test]
+fn test_mis_simple_one_to_triangular_is_deterministic_on_large_graph() {
+    // Regression for #1061: the triangular mapping shares the same greedy
+    // path decomposition as the KSG mapping, so it had the same non-determinism
+    // (unseeded RNG + HashSet iteration order in `adj`). A 64-vertex graph
+    // forces `pathwidth` to pick the Greedy branch.
+    let n = 64;
+    let mut edges: Vec<(usize, usize)> = Vec::new();
+    for r in 0..8 {
+        for c in 0..8 {
+            let v = r * 8 + c;
+            if c + 1 < 8 {
+                edges.push((v, r * 8 + c + 1));
+            }
+            if r + 1 < 8 {
+                edges.push((v, (r + 1) * 8 + c));
+            }
+            if r + 1 < 8 && c + 1 < 8 {
+                edges.push((v, (r + 1) * 8 + c + 1));
+            }
+        }
+    }
+
+    let problem = MaximumIndependentSet::new(SimpleGraph::new(n, edges), vec![One; n]);
+    let first = ReduceTo::<MaximumIndependentSet<TriangularSubgraph, i32>>::reduce_to(&problem);
+    let baseline_atoms = first.target_problem().graph().num_vertices();
+    let baseline_edges = first.target_problem().graph().edges().len();
+
+    for _ in 0..3 {
+        let again = ReduceTo::<MaximumIndependentSet<TriangularSubgraph, i32>>::reduce_to(&problem);
+        assert_eq!(
+            again.target_problem().graph().num_vertices(),
+            baseline_atoms,
+        );
+        assert_eq!(again.target_problem().graph().edges().len(), baseline_edges);
+    }
+}
+
 #[test]
 fn test_mis_simple_one_to_triangular_closed_loop() {
     // Path graph: 0-1-2

src/unit_tests/rules/unitdiskmapping/pathdecomposition.rs

@@ -219,6 +219,66 @@ fn test_pathwidth_auto_large() {
     assert_eq!(layout.vsep(), 1); // Path graph has pathwidth 1
 }
 
+#[test]
+fn test_pathwidth_greedy_is_deterministic() {
+    // Reproduces issue #1061: pathwidth greedy must be deterministic across calls.
+    // Graph is large enough that Auto picks Greedy and tie-breaking in greedy_step
+    // is exercised (grid with many symmetric choices).
+    let n = 64;
+    let mut edges: Vec<(usize, usize)> = Vec::new();
+    for r in 0..8 {
+        for c in 0..8 {
+            let v = r * 8 + c;
+            if c + 1 < 8 {
+                edges.push((v, r * 8 + c + 1));
+            }
+            if r + 1 < 8 {
+                edges.push((v, (r + 1) * 8 + c));
+            }
+        }
+    }
+
+    let first = pathwidth(n, &edges, PathDecompositionMethod::Auto);
+    for _ in 0..3 {
+        let other = pathwidth(n, &edges, PathDecompositionMethod::Auto);
+        assert_eq!(other.vertices, first.vertices);
+        assert_eq!(other.vsep(), first.vsep());
+    }
+}
+
+#[test]
+fn test_pathwidth_with_seed_varies_output() {
+    // Different seeds may produce different layouts (same pathwidth or better/worse).
+    // This verifies the seed API actually affects tie-breaking.
+    let n = 64;
+    let mut edges: Vec<(usize, usize)> = Vec::new();
+    for r in 0..8 {
+        for c in 0..8 {
+            let v = r * 8 + c;
+            if c + 1 < 8 {
+                edges.push((v, r * 8 + c + 1));
+            }
+            if r + 1 < 8 {
+                edges.push((v, (r + 1) * 8 + c));
+            }
+        }
+    }
+
+    let method = PathDecompositionMethod::greedy();
+    let a = pathwidth_with_seed(n, &edges, method, 0);
+    let b = pathwidth_with_seed(n, &edges, method, 0);
+    assert_eq!(a.vertices, b.vertices, "same seed → same layout");
+
+    // At least one of the sampled seeds should produce a different ordering,
+    // confirming the seed actually threads into tie-breaking.
+    let differs =
+        (1u64..=10).any(|s| pathwidth_with_seed(n, &edges, method, s).vertices != a.vertices);
+    assert!(
+        differs,
+        "expected some seed in 1..=10 to produce a different vertex ordering than seed 0",
+    );
+}
+
 // === Ground truth tests from JSON dataset ===
 
 /// Compute vsep from scratch for a given vertex ordering on a graph.