gate: confirm blocking regressions at fresh seeds before they block

MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py

@@ -38,6 +38,71 @@ def _summ(r):
                 neff=r["n_eff"])
 
 
+def classify(b, c):
+    """Return (verdict, note) for one base/candidate record pair.
+
+    SINGLE SOURCE OF TRUTH for what counts as a regression.  confirm_regressions.py
+    imports this: it previously reimplemented only the PASS->non-PASS case and was blind to
+    REGRESSION(metrics), so a real metric regression (measured: n_eff 448->210) produced
+    "no blocking regressions to confirm" and exited 0.  Two copies of this logic will always
+    drift; there is now one.
+    """
+    if c is None and b is not None:
+        # The candidate produced NO record for a row the base did.  That is a regression, not a
+        # bookkeeping curiosity: a candidate that crashes before emitting a result would otherwise
+        # be classified ONLY-IN-BASE, never reach confirmation, and exit the gate successfully --
+        # bypassing the fail-closed rerun logic entirely.
+        return "REGRESSION(missing-in-candidate)", "candidate produced no record for this row"
+    if b is None:
+        return "ONLY-IN-CANDIDATE", ""
+    st_b, _ = evaluate(b)
+    st_c, why_c = evaluate(c)
+    sb, sc = _summ(b), _summ(c)
+    verdict, note = "OK", ""
+    if st_b == "PASS" and st_c != "PASS":
+        # includes healthy->STARVED: candidate lost the efficiency the
+        # base had on this target -> regression
+        verdict = "REGRESSION(pass->{})".format(st_c.lower())
+        note = "; ".join(why_c)
+    elif st_b != "PASS" and st_c == "PASS":
+        verdict = "IMPROVED({}->pass)".format(st_b.lower())
+    elif st_b == "STARVED" and st_c == "STARVED":
+        verdict = "BOTH-STARVED"
+    elif st_b == "STARVED" and st_c in ("FAIL", "ERROR"):
+        # base gave no shape information here; candidate at least reaches
+        # testability (or crashes) -- flag, don't block
+        verdict = "NEWLY-TESTABLE-" + st_c
+        note = "; ".join(why_c)
+    elif st_b in ("FAIL", "ERROR") and st_c != "PASS":
+        verdict = "PREEXISTING-FAIL"
+    elif sb and sc:
+        worse = []
+        for m, tol in TOL_WORSE.items():
+            if m == "neff_frac":
+                if sc["neff"] < TOL_WORSE["neff_frac"] * sb["neff"]:
+                    worse.append("n_eff {:.0f}->{:.0f}".format(sb["neff"], sc["neff"]))
+            elif sc[m] - sb[m] > tol:
+                worse.append("{} {:.3f}->{:.3f}".format(m, sb[m], sc[m]))
+        if worse:
+            verdict = "REGRESSION(metrics)"
+            note = "; ".join(worse)
+    return verdict, note
+
+
+def is_blocking(verdict, kind, strict):
+    return verdict.startswith("REGRESSION") and kind in strict
+
+
+def blocking_keys(base, cand, strict):
+    """Every (kind, target) the gate would BLOCK on -- both regression flavours."""
+    out = []
+    for k in sorted(set(base) | set(cand)):
+        v, _ = classify(base.get(k), cand.get(k))
+        if is_blocking(v, k[0], strict):
+            out.append(k)
+    return out
+
+
 def main():
     ap = argparse.ArgumentParser()
     ap.add_argument("base")
@@ -46,6 +111,15 @@ def main():
     # regression there must block.  Note the intended asymmetry on first merge -- portfolio_seq
     # FAILs on a base without clear_warm_state and PASSes here, i.e. IMPROVED (non-blocking).
     # Its value is forward-looking: once this is the base, re-breaking the reset blocks.
+    # ENFORCEMENT.  Given both checkouts, a blocking regression is re-tested at fresh seeds
+    # before it is allowed to fail the gate, and THIS script's exit code reflects the confirmed
+    # verdict.  Without these the script only reports, and confirmation is advisory -- which is
+    # how the first version shipped: documented in the runner but never actually invoked.
+    ap.add_argument("--confirm-base-checkout", default=None,
+                    help="with --confirm-cand-checkout: re-test blocking rows at fresh seeds")
+    ap.add_argument("--confirm-cand-checkout", default=None)
+    ap.add_argument("--confirm-repeats", type=int, default=5)
+    ap.add_argument("--confirm-jobs", type=int, default=4)
     ap.add_argument("--strict-samplers",
                     default="AV,GMM,portfolio_warm,portfolio_seq,portfolio_seq_nobs")
     opts = ap.parse_args()
@@ -59,42 +133,8 @@ def main():
     n_block = 0
     rows = []
     for k in sorted(set(base) | set(cand)):
-        b, c = base.get(k), cand.get(k)
-        if b is None or c is None:
-            rows.append((k, "ONLY-IN-" + ("CANDIDATE" if b is None else "BASE"), ""))
-            continue
-        st_b, _ = evaluate(b)
-        st_c, why_c = evaluate(c)
-        sb, sc = _summ(b), _summ(c)
-        verdict, note = "OK", ""
-        if st_b == "PASS" and st_c != "PASS":
-            # includes healthy->STARVED: candidate lost the efficiency the
-            # base had on this target -> regression
-            verdict = "REGRESSION(pass->{})".format(st_c.lower())
-            note = "; ".join(why_c)
-        elif st_b != "PASS" and st_c == "PASS":
-            verdict = "IMPROVED({}->pass)".format(st_b.lower())
-        elif st_b == "STARVED" and st_c == "STARVED":
-            verdict = "BOTH-STARVED"
-        elif st_b == "STARVED" and st_c in ("FAIL", "ERROR"):
-            # base gave no shape information here; candidate at least reaches
-            # testability (or crashes) -- flag, don't block
-            verdict = "NEWLY-TESTABLE-" + st_c
-            note = "; ".join(why_c)
-        elif st_b in ("FAIL", "ERROR") and st_c != "PASS":
-            verdict = "PREEXISTING-FAIL"
-        elif sb and sc:
-            worse = []
-            for m, tol in TOL_WORSE.items():
-                if m == "neff_frac":
-                    if sc["neff"] < TOL_WORSE["neff_frac"] * sb["neff"]:
-                        worse.append("n_eff {:.0f}->{:.0f}".format(sb["neff"], sc["neff"]))
-                elif sc[m] - sb[m] > tol:
-                    worse.append("{} {:.3f}->{:.3f}".format(m, sb[m], sc[m]))
-            if worse:
-                verdict = "REGRESSION(metrics)"
-                note = "; ".join(worse)
-        blocking = verdict.startswith("REGRESSION") and k[0] in strict
+        verdict, note = classify(base.get(k), cand.get(k))
+        blocking = is_blocking(verdict, k[0], strict)
         if blocking:
             n_block += 1
         rows.append((k, verdict + ("  <-- BLOCKS MERGE" if blocking else ""), note))
@@ -103,7 +143,23 @@ def main():
         print("{:<10s} {:<16s} {} {}".format(kind, tgt, verdict,
                                              ("[" + note + "]") if note else ""))
     print("# blocking regressions (strict={}): {}".format(sorted(strict), n_block))
-    return 1 if n_block else 0
+    if not n_block:
+        return 0
+    if not (opts.confirm_base_checkout and opts.confirm_cand_checkout):
+        print("# NOT CONFIRMED AT FRESH SEEDS: pass --confirm-base-checkout/--confirm-cand-checkout\n"
+              "#   to re-test these rows before treating them as real.  Every threshold here is a\n"
+              "#   hard cut on a stochastic quantity, so a single blocking row is a hypothesis.")
+        return 1
+    import confirm_regressions
+    print("\n# re-testing {} blocking row(s) at {} fresh seeds per arm".format(
+        n_block, opts.confirm_repeats))
+    return confirm_regressions.main([
+        opts.base, opts.candidate,
+        "--base-checkout", opts.confirm_base_checkout,
+        "--cand-checkout", opts.confirm_cand_checkout,
+        "--repeats", str(opts.confirm_repeats),
+        "--jobs", str(opts.confirm_jobs),
+        "--strict-samplers", opts.strict_samplers])
 
 
 if __name__ == "__main__":

MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py

@@ -0,0 +1,181 @@
+#!/usr/bin/env python
+"""Re-test the blocking regressions a merge-gate comparison reported, at NEW random seeds.
+
+WHY THIS EXISTS.  Every gate verdict is a hard threshold (n_eff >= 100, JS < 3*floor + 0.004, ...)
+applied to a stochastic quantity, so a cell sitting near a threshold flips on realization alone.
+Observed: `GMM mix_d6_n3_s303` read n_eff 66 / 119 / 104 across runs of the SAME unchanged
+checkout -- straddling the 100 floor -- purely from where it landed in the worker pool.  Reported
+as a REGRESSION once, it would have blocked a merge that changed nothing about that sampler.
+
+The fix is NOT to make the samplers deterministic.  Independent copies that localize differently
+are our main detector for support/mode-collapse failures; pinning every fit to one seed would
+silence it, and would make N copies of a production run no better than one.  The fix is to ask the
+question again, properly: re-run the disputed cell in BOTH arms at several fresh run seeds and see
+whether the candidate is really worse.
+
+Usage:
+  confirm_regressions.py base.json cand.json --base-checkout DIR --cand-checkout DIR \\
+      [--repeats 3] [--jobs 4] [--seeds 11,22,33]
+
+Exit 0 if no regression is CONFIRMED; 1 if any is.  A regression is confirmed when the candidate
+is worse than the base in a MAJORITY of the fresh seeds (ties count as not-worse: the burden of
+proof is on the claim that the candidate broke something).
+"""
+import argparse
+import json
+import os
+import subprocess
+import sys
+import tempfile
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, HERE)
+# The comparator OWNS the definition of "blocking".  Importing it -- rather than reimplementing
+# the PASS->non-PASS case, as this script first did -- is what keeps the two in step: the local
+# copy was blind to REGRESSION(metrics), so a real metric regression (measured: n_eff 448->210)
+# reported "no blocking regressions to confirm" and exited 0.
+from compare_shape_results import classify, is_blocking, blocking_keys   # noqa: E402
+
+
+def _key(r):
+    return (r["kind"], r["target"])
+
+
+def _blocking(base_path, cand_path, strict):
+    with open(base_path) as fh:
+        base = {_key(r): r for r in json.load(fh)}
+    with open(cand_path) as fh:
+        cand = {_key(r): r for r in json.load(fh)}
+    return [(k, base.get(k), cand.get(k)) for k in blocking_keys(base, cand, strict)]
+
+
+def _rerun(checkout, rec, seed, jobs, tag):
+    """Re-run ONE cell of the matrix at a given run seed; return its record or None."""
+    fd, path = tempfile.mkstemp(suffix=".json", prefix="confirm_%s_" % tag)
+    os.close(fd)
+    cmd = [os.environ.get("PYTHON", "python3"), os.path.join(HERE, "shape_recovery.py"),
+           "--preset", "standard", "--json", path, "--jobs", str(jobs),
+           "--samplers", rec["kind"] if not rec["kind"].startswith(("portfolio_warm",
+                                                                   "portfolio_seq", "AV_seq"))
+           else "AV",
+           "--dims", str(rec["ndim"]), "--ncomps", str(rec["ncomp"]),
+           "--target-seeds", str(rec["target_seed"]), "--run-seed", str(seed),
+           "--warm-cases", "on" if rec["kind"] in ("portfolio_warm", "portfolio_seq",
+                                                   "portfolio_seq_nobs", "AV_seq") else "off"]
+    env = dict(os.environ)
+    env["PYTHONPATH"] = os.path.join(checkout, "MonteCarloMarginalizeCode", "Code") + \
+        os.pathsep + env.get("PYTHONPATH", "")
+    env["CUDA_VISIBLE_DEVICES"] = ""
+    try:
+        subprocess.run(cmd, env=env, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
+                       check=False)
+        with open(path) as fh:
+            for r in json.load(fh):
+                if _key(r) == _key(rec):
+                    return r
+    except Exception:
+        return None
+    finally:
+        try:
+            os.unlink(path)
+        except OSError:
+            pass
+    return None
+
+
+def main(argv=None):
+    ap = argparse.ArgumentParser(description=__doc__.split("\n")[1])
+    ap.add_argument("base")
+    ap.add_argument("candidate")
+    ap.add_argument("--base-checkout", required=True)
+    ap.add_argument("--cand-checkout", required=True)
+    ap.add_argument("--repeats", type=int, default=3,
+                    help="fresh run seeds per arm (default 3; use more for a near-threshold cell)")
+    ap.add_argument("--seeds", default=None, help="explicit comma list, overrides --repeats")
+    ap.add_argument("--jobs", type=int, default=4)
+    ap.add_argument("--min-valid", type=int, default=None,
+                    help="usable base/candidate pairs required for a verdict (default: all "
+                         "seeds). Fewer -> INCONCLUSIVE and exit 1, never a silent clear.")
+    ap.add_argument("--strict-samplers",
+                    default="AV,GMM,portfolio_warm,portfolio_seq,portfolio_seq_nobs")
+    opts = ap.parse_args(argv)
+
+    strict = set(x.strip() for x in opts.strict_samplers.split(",") if x.strip())
+    seeds = ([int(x) for x in opts.seeds.split(",")] if opts.seeds
+             else [987654 + 1000 * (i + 1) for i in range(opts.repeats)])
+    if opts.min_valid is None:
+        opts.min_valid = len(seeds)
+
+    disputed = _blocking(opts.base, opts.candidate, strict)
+    if not disputed:
+        print("# no blocking regressions to confirm")
+        return 0
+    print("# confirming {} blocking regression(s) at {} fresh seed(s): {}".format(
+        len(disputed), len(seeds), seeds))
+
+    n_confirmed = 0
+    n_inconclusive = 0
+    for k, brec, crec in disputed:
+        worse = same = 0
+        detail = []
+        for s in seeds:
+            # The cell to re-run is defined by whichever record exists -- for a
+            # REGRESSION(missing-in-candidate) row the candidate has no record, but the base
+            # record still tells us which (kind, dim, ncomp, seed) to run, so the candidate CAN
+            # and must be re-tested rather than written off.
+            spec = brec if brec is not None else crec
+            rb = _rerun(opts.base_checkout, spec, s, opts.jobs, "base")
+            rc = _rerun(opts.cand_checkout, spec, s, opts.jobs, "cand")
+            if rc is None and rb is None:
+                detail.append("seed {}: BOTH reruns produced no record (no evidence either way)"
+                              .format(s))
+                continue
+            if rc is None:
+                # The CANDIDATE failed where the base did not.  That is not missing evidence, it
+                # IS the regression: crashing or emitting no record is worse than passing.
+                # Discarding it -- as this script first did -- let a candidate that failed on
+                # every seed be declared "not confirmed".
+                worse += 1
+                detail.append("seed {}: CANDIDATE PRODUCED NO RECORD (counts against candidate)"
+                              .format(s))
+                continue
+            if rb is None:
+                detail.append("seed {}: base rerun produced no record; pair unusable".format(s))
+                continue
+            # the SAME classifier the gate uses, so a metrics-only regression is judged here
+            # exactly as it was there
+            verdict, note = classify(rb, rc)
+            if is_blocking(verdict, k[0], strict):
+                worse += 1
+            else:
+                same += 1
+            detail.append("seed {}: {} (n_eff {:.0f} vs {:.0f}){}".format(
+                s, verdict, rb.get("n_eff", float("nan")), rc.get("n_eff", float("nan")),
+                "  [" + note + "]" if note else ""))
+
+        valid = worse + same
+        if valid < opts.min_valid:
+            status = ("INCONCLUSIVE -- {}/{} valid pairs, need {}: NOT cleared"
+                      .format(valid, len(seeds), opts.min_valid))
+            n_inconclusive += 1
+        elif worse > same:
+            status = "CONFIRMED REGRESSION -- BLOCKS ({} worse / {} not-worse)".format(worse, same)
+            n_confirmed += 1
+        else:
+            status = ("NOT CONFIRMED (realization noise; does not block) ({} worse / {} not-worse)"
+                      .format(worse, same))
+        print("\n{} {}".format(k[0], k[1]))
+        for d in detail:
+            print("   " + d)
+        print("   -> " + status)
+
+    print("\n# confirmed blocking regressions: {}".format(n_confirmed))
+    if n_inconclusive:
+        print("# INCONCLUSIVE rows (too few valid reruns): {}".format(n_inconclusive))
+    # Inconclusive must NOT read as success: we failed to obtain the evidence that would clear
+    # the row, so the gate stays red until a human looks.
+    return 1 if (n_confirmed or n_inconclusive) else 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh

@@ -1,6 +1,31 @@
 #!/bin/bash
 # Standard merge-gate invocation of the shape-recovery suite.
 #
+# A FAIL FROM THIS SCRIPT IS NOT A VERDICT.  Every gate threshold (n_eff >= 100, JS, pull, width)
+# is a hard cut on a stochastic quantity, so any cell sitting near a threshold flips on
+# realization alone.  Before treating a blocking regression as real, re-test it at fresh seeds:
+#
+#     compare_shape_results.py base.json cand.json \
+#         --confirm-base-checkout DIR --confirm-cand-checkout DIR --confirm-repeats 5
+#
+# With those flags the comparison ENFORCES confirmation: blocking rows are re-tested and the exit
+# code is the confirmed verdict.  Without them it still exits 1 on a blocking row, but says so
+# explicitly rather than implying the row was confirmed.  (confirm_regressions.py also runs
+# standalone against an existing pair of JSONs.)
+#
+# It re-runs only the disputed cells, in BOTH arms, at several new run seeds, and blocks only if
+# the candidate is worse in a majority.  A candidate that produces NO record where the base did
+# counts against the candidate, and too few usable pairs is INCONCLUSIVE (exit 1), never a clear.  Worked example: `GMM mix_d6_n3_s303` was reported as a
+# blocking REGRESSION in two consecutive full runs (base 119, candidate 66) and looked
+# reproducible -- but at 5 fresh seeds the two arms were BIT-IDENTICAL (93/93, 80/80, 119/119,
+# 95/95, 96/96) and 4 of the 5 starved.  The cell simply sits on the n_eff=100 floor; its PASS at
+# the default seed was the lucky draw, and the apparent regression was an artifact of where the
+# job landed in the worker pool.
+#
+# Do NOT "fix" this by seeding the samplers deterministically.  Independent copies that localize
+# differently are the working detector for support/mode-collapse failures; pinning every fit to
+# one seed silences it, and makes N production copies no better than one.
+#
 #   ./run_shape_recovery.sh /path/to/checkout results.json [extra args...]
 #
 # Runs CPU-only (deterministic; also exercises the cupy-installed-but-no-GPU