fp-stability: address PR review — silent failures, 1-row crash, dead code, tests, comment rot

From a multi-agent PR review:

- Silent failures (critical): a crashed cancellation/float-max/MCA run was reported as a clean result ('none detected' / 'no overflows' / 'dev=0.0'). The run helpers now log the failure and return None (cancellation/float-max) or a completed-sample count (MCA, distinct from measured zero); _run_case reports 'run failed' instead of a false all-clear.

- Crash (important): np.loadtxt(...)[:,1] raised IndexError on a single-row .dat (reachable via a 1-cell user case), aborting the whole suite. Added _dat_column using np.atleast_2d; also fixed the generated dd_cmp.py oracle.

- Dead code: removed _cancellation_by_file (superseded by the digits-lost severity view) and _stability_pass (the orchestrator inlines the comparison), plus their tests.

- Tests: smoke tests for the CI-only report emitters (blank + populated result, and unconfirmed->::notice:: downgrade), _digits_left clamp, and #:block/#:call/unbalanced macro cases.

- Comment rot: stage-E docstring now says confirmation is set-level (not per-line); dropped the non-existent 'per-file density' reference; fixed the '48 ~ full mantissa' note (53-bit). 45 tests, ruff, precheck all 7.

Author: sbryngelson

toolchain/mfc/fp_stability.py

@@ -24,13 +24,14 @@
    Each reported line is then *confirmed* by a positive control: --gen-source
    captures the symbol-correct executed lines, those are filtered to the suspect
    set, and a float-mode run with --source restricted to just them must
-   reproduce the instability.  Lines that do not reproduce it are reported as
-   unconfirmed (downgraded from ::warning:: to ::notice::).  Each line is then
-   perturbed alone and ranked by the share of the single-precision deviation it
-   reproduces.  NOTE: this is a *sensitivity* measure — where reduced precision
-   most moves the output — and is typically dominated by the time integrator /
-   final accumulation, NOT by where cancellation originates.  Stage F (and its
-   per-file density) is the cancellation-origin view; the two usually differ.
+   reproduce the instability.  If perturbing the suspect set does not reproduce
+   it, the case's hotspots are reported as unconfirmed (downgraded from
+   ::warning:: to ::notice::) — this is a single set-level verdict, not per line.
+   Each line is then perturbed alone and ranked by the share of the single-
+   precision deviation it reproduces.  NOTE: that share is a *sensitivity*
+   measure — where reduced precision most moves the output — typically dominated
+   by the time integrator / final accumulation, NOT by where cancellation
+   originates.  Stage F is the cancellation-origin view; the two usually differ.
    Hotspots are cross-referenced against the stage-F cancellation sites and
    flagged as instance-ambiguous when the .fpp line sits inside a #:for/#:def
    expansion.
@@ -550,35 +551,42 @@ def _run_case(
         if run_cancellation:
             cons.print("  [dim]cancellation detection...[/dim]")
             try:
-                # sweep bit thresholds to get per-site severity (bits lost)
+                # sweep bit thresholds to get per-site severity (bits lost); each
+                # run returns None if it failed (distinct from [] = ran, found none)
                 level_sites = [(level, _run_cancellation_check(verrou_bin, sim_bin, work_dir, threshold=level)) for level in CANCEL_BIT_LEVELS]
-                locs = level_sites[0][1]  # lowest threshold = full list
-                bits = _cancellation_severity(level_sites)
-                result["cancellation_locs"] = locs
-                result["cancellation_bits"] = bits
-                if locs:
-                    worst = max(bits.values()) if bits else 0
-                    cons.print(f"  cancellation: {len(locs)} site(s), worst loses ≥ {worst / math.log2(10):.0f} of ~16 digits")
+                locs = next((s for lvl, s in level_sites if lvl == CANCEL_BIT_LEVELS[0]), None)
+                if locs is None:
+                    cons.print("  [bold yellow]cancellation: detection run failed (see logs); not reported[/bold yellow]")
                 else:
-                    cons.print("  cancellation: none detected")
-                # cross-reference: label dd_line hotspots that sit on a cancellation site
-                if result["dd_line_locs"] and locs:
-                    _mark_cancellation(result["dd_line_locs"], locs)
-                    n_xref = sum(1 for loc in result["dd_line_locs"] if loc.get("cancellation"))
-                    if n_xref:
-                        cons.print(f"  {n_xref} hotspot(s) coincide with a catastrophic-cancellation site")
+                    bits = _cancellation_severity([(lvl, s) for lvl, s in level_sites if s is not None])
+                    result["cancellation_locs"] = locs
+                    result["cancellation_bits"] = bits
+                    if locs:
+                        worst = max(bits.values()) if bits else 0
+                        cons.print(f"  cancellation: {len(locs)} site(s), worst loses ≥ {worst / math.log2(10):.0f} of ~16 digits")
+                    else:
+                        cons.print("  cancellation: none detected")
+                    # cross-reference: label dd_line hotspots that sit on a cancellation site
+                    if result["dd_line_locs"] and locs:
+                        _mark_cancellation(result["dd_line_locs"], locs)
+                        n_xref = sum(1 for loc in result["dd_line_locs"] if loc.get("cancellation"))
+                        if n_xref:
+                            cons.print(f"  {n_xref} hotspot(s) coincide with a catastrophic-cancellation site")
             except Exception as exc:
                 cons.print(f"  [bold yellow]cancellation check error[/bold yellow]: {exc}")
 
         # --- G: MCA significant-bits estimate ---
         if run_mca:
             cons.print(f"  [dim]MCA significant-bits estimate (N={n_samples})...[/dim]")
             try:
-                mca_dev, mca_sigbits = _run_mca_samples(case, verrou_bin, sim_bin, work_dir, ref_dir, n_samples)
-                result["mca_dev"] = mca_dev
-                result["mca_sigbits"] = mca_sigbits
-                bits_str = f"~{mca_sigbits} sig bits" if mca_sigbits is not None else "n/a"
-                cons.print(f"  MCA: dev={mca_dev:.3e}  ({bits_str})")
+                mca_dev, mca_sigbits, n_ok = _run_mca_samples(case, verrou_bin, sim_bin, work_dir, ref_dir, n_samples)
+                if n_ok == 0:
+                    cons.print(f"  [bold yellow]MCA: no samples completed (0/{n_samples}; see logs)[/bold yellow]")
+                else:
+                    result["mca_dev"] = mca_dev
+                    result["mca_sigbits"] = mca_sigbits
+                    bits_str = f"~{mca_sigbits} sig bits" if mca_sigbits is not None else "n/a"
+                    cons.print(f"  MCA: dev={mca_dev:.3e}  ({bits_str})  [{n_ok}/{n_samples} samples]")
             except Exception as exc:
                 cons.print(f"  [bold yellow]MCA error[/bold yellow]: {exc}")
 
@@ -587,11 +595,14 @@ def _run_case(
             cons.print("  [dim]float-max overflow check...[/dim]")
             try:
                 locs = _run_float_max_check(verrou_bin, sim_bin, work_dir)
-                result["float_max_locs"] = locs
-                if locs:
-                    cons.print(f"  [bold yellow]float-max[/bold yellow]: {len(locs)} overflow site(s)")
+                if locs is None:
+                    cons.print("  [bold yellow]float-max: run failed (see logs); not reported[/bold yellow]")
                 else:
-                    cons.print("  float-max: no overflows")
+                    result["float_max_locs"] = locs
+                    if locs:
+                        cons.print(f"  [bold yellow]float-max[/bold yellow]: {len(locs)} overflow site(s)")
+                    else:
+                        cons.print("  float-max: no overflows")
             except Exception as exc:
                 cons.print(f"  [bold yellow]float-max check error[/bold yellow]: {exc}")
 

toolchain/mfc/fp_stability_metrics.py

@@ -58,11 +58,6 @@ def _sig_bits(max_dev: float, ref_scale: float) -> float:
     return -math.log2(max_dev / ref_scale)
 
 
-def _stability_pass(max_dev: float, ref_scale: float, floor: float) -> bool:
-    """A case passes when it retains at least `floor` significant bits."""
-    return _sig_bits(max_dev, ref_scale) >= floor
-
-
 # Matches "path/file.f90:123" or "path/file.fpp:123-456" in dd_line rddmin_summary.
 _LOC_RE = re.compile(r"(\S+\.(?:f90|fpp|c|cpp|h|F90))\s*:(\d+)(?:-(\d+))?", re.IGNORECASE)
 
@@ -232,6 +227,14 @@ def _get_source_context(fname: str, lineno: int, context: int = 2) -> str:
     return "\n".join(rows)
 
 
+def _dat_column(path: str):
+    """Load column 1 (the field value) from an MFC .dat file, robust to a
+    single-row file (np.loadtxt returns 1-D then, which [:, 1] would crash on)."""
+    import numpy as np
+
+    return np.atleast_2d(np.loadtxt(path))[:, 1]
+
+
 def _max_diff_np(ref_dir: str, run_dir: str, compare_files: list) -> float:
     import numpy as np
 
@@ -240,9 +243,7 @@ def _max_diff_np(ref_dir: str, run_dir: str, compare_files: list) -> float:
         ref_p, run_p = os.path.join(ref_dir, fname), os.path.join(run_dir, fname)
         if not os.path.exists(ref_p) or not os.path.exists(run_p):
             return float("inf")
-        ref = np.loadtxt(ref_p)[:, 1]
-        run = np.loadtxt(run_p)[:, 1]
-        total = max(total, float(np.max(np.abs(ref - run))))
+        total = max(total, float(np.max(np.abs(_dat_column(ref_p) - _dat_column(run_p)))))
     return total
 
 
@@ -255,8 +256,7 @@ def _max_abs_np(ref_dir: str, compare_files: list) -> float:
         ref_p = os.path.join(ref_dir, fname)
         if not os.path.exists(ref_p):
             continue
-        ref = np.loadtxt(ref_p)[:, 1]
-        total = max(total, float(np.max(np.abs(ref))))
+        total = max(total, float(np.max(np.abs(_dat_column(ref_p)))))
     return total
 
 
@@ -321,7 +321,8 @@ def _parse_vg_error_locs(log_path: str, error_keyword: str) -> list:
 # Verrou exposes no per-site bit-count, but --cc-threshold-double is a severity
 # filter: a site is reported only if it lost >= the threshold bits. Sweeping these
 # levels and taking the highest each site survives gives a per-site "bits lost"
-# severity (a lower bound — no false positives). 48 ~ full double mantissa.
+# severity (a lower bound — no false positives). 48 is near the full 53-bit
+# double mantissa (the top of the sweep), not the mantissa width itself.
 CANCEL_BIT_LEVELS = [10, 20, 30, 40, 48]
 
 
@@ -474,19 +475,3 @@ def _mark_cancellation(dd_line_locs: list, cancellation_locs: list) -> list:
         lines = by_base.get(os.path.basename(loc["path"]), set())
         loc["cancellation"] = any(ln in lines for ln in range(loc["start"], loc["end"] + 1))
     return dd_line_locs
-
-
-def _cancellation_by_file(cancellation_locs: list) -> list:
-    """Aggregate cancellation sites by source file → [(basename, count)] sorted by
-    count (desc), ties by name.
-
-    This is the cancellation-*origin* view (where ill-conditioning concentrates),
-    as opposed to the per-line --source share, which is a *sensitivity* view
-    (where reduced precision most moves the output — typically the time
-    integrator / final accumulation, regardless of where error originates).
-    """
-    counts = {}
-    for fname, _lineno in cancellation_locs:
-        base = os.path.basename(fname)
-        counts[base] = counts.get(base, 0) + 1
-    return sorted(counts.items(), key=lambda kv: (-kv[1], kv[0]))

toolchain/mfc/fp_stability_runners.py

@@ -118,9 +118,10 @@ def _run_simulation_verrou(
             shutil.copy2(os.path.join(tmpdir, "D", fn), run_dir)
 
 
-def _run_cancellation_check(verrou_bin: str, sim_bin: str, work_dir: str, threshold: int = 10) -> list:
+def _run_cancellation_check(verrou_bin: str, sim_bin: str, work_dir: str, threshold: int = 10):
     """Run --check-cancellation at the given bit threshold; return [(fname, line)]
-    of MFC cancellation sites (subtractions losing >= `threshold` significant bits)."""
+    of MFC cancellation sites (subtractions losing >= `threshold` significant bits),
+    or None if the run itself failed (distinct from [] = ran and found none)."""
     tag = f"cancellation_{threshold}"
     run_dir = os.path.join(work_dir, tag)
     os.makedirs(run_dir, exist_ok=True)
@@ -132,8 +133,9 @@ def _run_cancellation_check(verrou_bin: str, sim_bin: str, work_dir: str, thresh
     ]
     try:
         _run_simulation_verrou(verrou_bin, sim_bin, work_dir, run_dir, rounding_mode="nearest", extra_flags=flags)
-    except MFCException:
-        pass
+    except MFCException as exc:
+        cons.print(f"  [yellow]cancellation run (threshold {threshold}) failed: {exc}[/yellow]")
+        return None
     raw = _parse_cancel_gen(gen_path)
     filtered = [(f, ln) for f, ln in raw if _is_arithmetic_loc(f, ln, ln)]
     skipped = len(raw) - len(filtered)
@@ -150,27 +152,31 @@ def _run_mca_samples(
     ref_dir: str,
     n_mca: int,
 ) -> tuple:
-    """Run N mcaquad samples; return (max_dev, sig_bits_lower_bound)."""
+    """Run N mcaquad samples; return (max_dev, sig_bits_lower_bound, n_ok) where
+    n_ok is how many samples actually completed (0 => no usable measurement)."""
     compare = case["compare"]
     ref_scale = _max_abs_np(ref_dir, compare)
     max_dev = 0.0
+    n_ok = 0
     flags = ["--backend=mcaquad", "--mca-mode=mca"]
     for i in range(n_mca):
         run_dir = os.path.join(work_dir, f"mca_{i:02d}")
         os.makedirs(run_dir, exist_ok=True)
         try:
             _run_simulation_verrou(verrou_bin, sim_bin, work_dir, run_dir, extra_flags=flags)
             max_dev = max(max_dev, _max_diff_np(ref_dir, run_dir, compare))
-        except MFCException:
-            pass
+            n_ok += 1
+        except MFCException as exc:
+            cons.print(f"  [dim]MCA sample {i} failed: {exc}[/dim]")
     sig_bits = None
-    if max_dev > 0.0 and ref_scale > 0.0:
+    if n_ok and max_dev > 0.0 and ref_scale > 0.0:
         sig_bits = max(0, int(math.floor(-math.log2(max_dev / ref_scale))))
-    return max_dev, sig_bits
+    return max_dev, sig_bits, n_ok
 
 
-def _run_float_max_check(verrou_bin: str, sim_bin: str, work_dir: str) -> list:
-    """Run with --check-max-float=yes; return [(fname, line)] of overflow sites."""
+def _run_float_max_check(verrou_bin: str, sim_bin: str, work_dir: str):
+    """Run with --check-max-float=yes; return [(fname, line)] of overflow sites,
+    or None if the run failed (distinct from [] = ran and found none)."""
     run_dir = os.path.join(work_dir, "float_max")
     os.makedirs(run_dir, exist_ok=True)
     try:
@@ -182,8 +188,9 @@ def _run_float_max_check(verrou_bin: str, sim_bin: str, work_dir: str) -> list:
             rounding_mode="nearest",
             extra_flags=["--check-max-float=yes"],
         )
-    except MFCException:
-        pass
+    except MFCException as exc:
+        cons.print(f"  [yellow]float-max run failed: {exc}[/yellow]")
+        return None
     return _parse_vg_error_locs(os.path.join(run_dir, "verrou.log"), "Max float")
 
 
@@ -291,8 +298,8 @@ def _write_dd_cmp_py(path: str, compare_files: list, threshold: float):
             if not os.path.exists(ref_p) or not os.path.exists(run_p):
                 print(f"MISSING: {{fname}}")
                 sys.exit(1)
-            ref = np.loadtxt(ref_p)[:, 1]
-            run = np.loadtxt(run_p)[:, 1]
+            ref = np.atleast_2d(np.loadtxt(ref_p))[:, 1]
+            run = np.atleast_2d(np.loadtxt(run_p))[:, 1]
             dev = float(np.max(np.abs(ref - run)))
             max_dev = max(max_dev, dev)