diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py index f3ac86a9e..1d2bf33d2 100755 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py +++ b/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__": diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py new file mode 100644 index 000000000..15224582e --- /dev/null +++ b/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()) diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh index 3675f4cde..95c870fef 100755 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh +++ b/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 diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py new file mode 100644 index 000000000..7f985b603 --- /dev/null +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python +"""Unit tests for the confirm-on-fail accounting. + +These cover the ways a confirmation step can WRONGLY CLEAR a real regression, which is the only +dangerous direction: a false block costs a rerun, a false clear ships a bug. + +Run: python test_confirm_regressions.py +""" +import sys + +import confirm_regressions as CR +from compare_shape_results import classify, is_blocking + +STRICT = {"GMM", "AV"} + + +def _rec(kind="GMM", target="t", n_eff=3000.0, js=0.0001, bias=0.001): + return dict(kind=kind, target=target, ndim=4, ncomp=1, target_seed=101, n_eff=n_eff, + n_ess=n_eff * 3, js=[js, js], js_floor=[0.0005, 0.0005], + mean_pull=[0.005, 0.005], width_ratio=[1.001, 1.001], corr_diff_max=0.005, + rel_err=0.01, bias_ln=bias, error=None) + + +def test_metrics_only_regression_is_recognised(): + """The comparator blocks on REGRESSION(metrics) too. A confirm step that only knew about + PASS->non-PASS reported 'nothing to confirm' and exited 0 on a real n_eff collapse.""" + b, c = _rec(n_eff=4000.0), _rec(n_eff=400.0) # 10x n_eff drop, both still PASS + verdict, _ = classify(b, c) + assert verdict == "REGRESSION(metrics)", verdict + assert is_blocking(verdict, "GMM", STRICT) + + +def _run_with(monkey_results, seeds=(1, 2, 3), min_valid=None): + """Drive main() with _rerun stubbed to a scripted sequence of (base, cand) records.""" + calls = {"i": 0} + + def fake_rerun(checkout, rec, seed, jobs, tag): + pair = monkey_results[calls["i"] // 2] + out = pair[0] if tag == "base" else pair[1] + calls["i"] += 1 + return out + + orig = CR._rerun + CR._rerun = fake_rerun + try: + import json, tempfile, os + b, c = _rec(n_eff=4000.0), _rec(n_eff=400.0) + paths = [] + for recs in ([b], [c]): + fd, p = tempfile.mkstemp(suffix=".json") + os.close(fd) + json.dump(recs, open(p, "w")) + paths.append(p) + argv = [paths[0], paths[1], "--base-checkout", "/b", "--cand-checkout", "/c", + "--seeds", ",".join(str(s) for s in seeds)] + if min_valid is not None: + argv += ["--min-valid", str(min_valid)] + return CR.main(argv) + finally: + CR._rerun = orig + + +def test_candidate_crash_counts_against_the_candidate(): + """If the candidate produces no record where the base does, that IS the regression. + Discarding those pairs let a candidate that failed on every seed be 'not confirmed'.""" + good = _rec(n_eff=4000.0) + rc = _run_with([(good, None), (good, None), (good, None)]) + assert rc == 1, "candidate produced no record on every seed but was cleared (rc={})".format(rc) + + +def test_insufficient_valid_pairs_is_inconclusive_not_a_pass(): + """Missing evidence must not read as 'cleared'.""" + good = _rec(n_eff=4000.0) + rc = _run_with([(None, None), (None, None), (None, None)]) + assert rc == 1, "zero valid pairs was reported as success (rc={})".format(rc) + + +def test_genuine_noise_clears(): + """A row that is equivalent at fresh seeds must clear, or the step is useless.""" + good = _rec(n_eff=4000.0) + rc = _run_with([(good, good), (good, good), (good, good)]) + assert rc == 0, "equivalent arms were reported as a confirmed regression (rc={})".format(rc) + + +def test_real_regression_is_confirmed(): + good, bad = _rec(n_eff=4000.0), _rec(n_eff=200.0) + rc = _run_with([(good, bad), (good, bad), (good, bad)]) + assert rc == 1, "a reproducible 20x n_eff drop was not confirmed (rc={})".format(rc) + + + + +def test_missing_candidate_record_is_a_blocking_regression(): + """A candidate that emits no record for a strict row must BLOCK. + + Classified as ONLY-IN-BASE it was not a regression, so it never reached confirmation and the + gate exited 0 -- a candidate crashing before its first result would bypass the fail-closed + rerun logic entirely.""" + b = _rec() + verdict, note = classify(b, None) + assert verdict.startswith("REGRESSION"), verdict + assert is_blocking(verdict, "GMM", STRICT), "missing candidate record did not block" + # and it must be picked up as a row to confirm + from compare_shape_results import blocking_keys + keys = blocking_keys({("GMM", "t"): b}, {}, STRICT) + assert keys == [("GMM", "t")], keys + + +def test_extra_candidate_record_is_not_a_regression(): + """The reverse direction is not a defect: a NEW row in the candidate must not block.""" + verdict, _ = classify(None, _rec()) + assert not verdict.startswith("REGRESSION"), verdict + + +if __name__ == "__main__": + for name, fn in sorted(globals().items()): + if name.startswith("test_"): + fn() + print("PASS", name) + print("confirm-on-fail accounting holds")