Skip to content

Commit bc7e516

Browse files
committed
fp-stability: distinguish precision-sensitivity from cancellation-origin; surface caps + coverage
The per-line --source ranking measures sensitivity (where reduced precision most moves the output), which is structurally dominated by the time integrator / final accumulation: perturbing the last write to q_cons hits the output 1:1, while upstream errors get re-rounded there. Empirically, sod_standard's cancellation concentrates in m_weno.fpp (14 sites) and m_riemann_solvers.fpp (5), with m_time_steppers.fpp at just 1 — yet the time-stepper led the share ranking at 100%. Presenting it as 'most flagrant' conflated sensitivity with where ill-conditioning originates. Reframe: the dd_line/share view is relabeled 'single-precision sensitivity' with an explicit caveat (typically the time integrator, expected/benign, not a cancellation-origin finder); a new per-file cancellation-density line (_cancellation_by_file) headlines where cancellation actually concentrates; console + GitHub summary + inline annotations updated to keep the two signals distinct. Also: no silent caps (truncated dd_line/cancellation/float-max lists now report '…and N more'; annotations emit a dropped-count notice), and a coverage caveat in the summary header (N 1-D cases; a pass is not a guarantee for unexercised multi-D/viscous/MHD/IGR/bubble paths). _cancellation_by_file is pure + TDD'd.
1 parent 196aff5 commit bc7e516

2 files changed

Lines changed: 89 additions & 12 deletions

File tree

toolchain/mfc/fp_stability.py

Lines changed: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,13 @@
2626
reproduce the instability. Lines that do not reproduce it are reported as
2727
unconfirmed (downgraded from ::warning:: to ::notice::). Each line is then
2828
perturbed alone and ranked by the share of the single-precision deviation it
29-
reproduces, so the most flagrant computation is identified rather than a flat
30-
list. Hotspots are additionally cross-referenced against the stage-F
31-
cancellation sites (to name the offending subtraction) and flagged as
32-
instance-ambiguous when the .fpp line sits inside a #:for/#:def expansion.
29+
reproduces. NOTE: this is a *sensitivity* measure — where reduced precision
30+
most moves the output — and is typically dominated by the time integrator /
31+
final accumulation, NOT by where cancellation originates. Stage F (and its
32+
per-file density) is the cancellation-origin view; the two usually differ.
33+
Hotspots are cross-referenced against the stage-F cancellation sites and
34+
flagged as instance-ambiguous when the .fpp line sits inside a #:for/#:def
35+
expansion.
3336
3437
F. Cancellation detection (--no-cancellation to skip)
3538
One run with --check-cancellation=yes; reports MFC source lines that
@@ -1003,6 +1006,22 @@ def _mark_cancellation(dd_line_locs: list, cancellation_locs: list) -> list:
10031006
return dd_line_locs
10041007

10051008

1009+
def _cancellation_by_file(cancellation_locs: list) -> list:
1010+
"""Aggregate cancellation sites by source file → [(basename, count)] sorted by
1011+
count (desc), ties by name.
1012+
1013+
This is the cancellation-*origin* view (where ill-conditioning concentrates),
1014+
as opposed to the per-line --source share, which is a *sensitivity* view
1015+
(where reduced precision most moves the output — typically the time
1016+
integrator / final accumulation, regardless of where error originates).
1017+
"""
1018+
counts = {}
1019+
for fname, _lineno in cancellation_locs:
1020+
base = os.path.basename(fname)
1021+
counts[base] = counts.get(base, 0) + 1
1022+
return sorted(counts.items(), key=lambda kv: (-kv[1], kv[0]))
1023+
1024+
10061025
def _run_dd_tool(
10071026
dd_bin: str,
10081027
dd_dir: str,
@@ -1361,7 +1380,9 @@ def _run_case(
13611380
cons.print(f" [bold yellow]dd_line UNCONFIRMED[/bold yellow]: suspect-only dev={cdev:.3e} < {dd_threshold:.1e} (attribution suspect)")
13621381
top = ranked[0] if ranked else None
13631382
if top and top.get("share") is not None:
1364-
cons.print(f" most flagrant: {top['path']}:{top['start']} ({top['share'] * 100:.0f}% of float-proxy)")
1383+
cons.print(f" highest single-precision sensitivity: {top['path']}:{top['start']} ({top['share'] * 100:.0f}% of float-proxy)")
1384+
cons.print(" [dim](sensitivity = where reduced precision most moves the output, often the time")
1385+
cons.print(" [dim] integrator; not necessarily where cancellation originates — see cancellation sites)[/dim]")
13651386
except Exception as exc:
13661387
cons.print(f" [bold yellow]dd_line confirmation error[/bold yellow]: {exc}")
13671388

@@ -1450,7 +1471,7 @@ def _emit_github_annotations(results: list):
14501471
if not os.environ.get("GITHUB_ACTIONS"):
14511472
return
14521473
for r in results:
1453-
status = "FAIL" if not r["passed"] else "hotspot"
1474+
status = "FAIL" if not r["passed"] else "sensitivity"
14541475
dev_str = f"max_dev={r['max_dev']:.2e} (threshold {r['threshold']:.0e})"
14551476
unconfirmed = r.get("dd_line_confirmed") is False
14561477

@@ -1460,9 +1481,9 @@ def _emit_github_annotations(results: list):
14601481
location += f",endLine={loc['end']}"
14611482
note = dev_str
14621483
if loc.get("share") is not None:
1463-
note += f" — reproduces {loc['share'] * 100:.0f}% of float-proxy alone"
1484+
note += f" — single-precision sensitivity: {loc['share'] * 100:.0f}% of float-proxy (where precision matters, not necessarily where cancellation originates)"
14641485
if loc.get("cancellation"):
1465-
note += " — catastrophic cancellation site"
1486+
note += " — also a catastrophic cancellation site"
14661487
if loc.get("macro"):
14671488
note += f" — {loc['macro']}-expanded line, may represent multiple instances"
14681489
if unconfirmed:
@@ -1471,11 +1492,17 @@ def _emit_github_annotations(results: list):
14711492
else:
14721493
title = f"FP {status} [{r['name']}]"
14731494
print(f"::warning {location},title={title}::{note}", flush=True)
1495+
n_dd = len(r.get("dd_line_locs", []))
1496+
if n_dd > 3:
1497+
print(f"::notice title=FP hotspots [{r['name']}]::{n_dd - 3} more dd_line hotspot(s) not annotated inline; see the step summary", flush=True)
14741498

14751499
for fname, lineno in r.get("cancellation_locs", [])[:3]:
14761500
loc = f"file={fname},line={lineno}"
14771501
title = f"FP cancellation [{r['name']}]"
14781502
print(f"::notice {loc},title={title}::catastrophic cancellation site", flush=True)
1503+
n_cc = len(r.get("cancellation_locs", []))
1504+
if n_cc > 3:
1505+
print(f"::notice title=FP cancellation [{r['name']}]::{n_cc - 3} more cancellation site(s) not annotated inline; see the step summary", flush=True)
14791506

14801507

14811508
def _emit_github_summary(results: list, n_samples: int):
@@ -1495,6 +1522,12 @@ def _emit_github_summary(results: list, n_samples: int):
14951522
md = []
14961523
md.append("## FP Stability Results\n")
14971524
md.append(f"**{n_pass} passed, {n_fail} failed** — {n_samples} random-rounding samples per case\n")
1525+
md.append(
1526+
f"> **Coverage:** {len(results)} one-dimensional case(s) "
1527+
f"({', '.join(r['name'] for r in results)}). A pass means stable in the code paths these "
1528+
"cases exercise — not a guarantee for multi-D, viscous, MHD, IGR, or bubble-dynamics paths "
1529+
"they do not reach.\n"
1530+
)
14981531

14991532
# Main results table
15001533
md.append("| Case | Status | max\\_dev | threshold | Float proxy | MCA sig bits |")
@@ -1528,10 +1561,19 @@ def _emit_github_summary(results: list, n_samples: int):
15281561
md.append(f"| `{r['name']}` | {' | '.join(cols)} |")
15291562
md.append("")
15301563

1531-
# dd_line hotspot sources — always shown (top 10 per case) with source context
1564+
# dd_line — single-precision SENSITIVITY (where precision most affects the
1565+
# output). This is distinct from cancellation origin (reported separately):
1566+
# the leader is typically the time integrator / final accumulation, because
1567+
# perturbing the last write moves the output directly while upstream errors
1568+
# get re-rounded there. Not a culprit-finder for ill-conditioning.
15321569
cases_with_locs = [r for r in results if r["dd_line_locs"]]
15331570
if cases_with_locs:
1534-
md.append("### Top FP hotspots (dd\\_line)\n")
1571+
md.append("### Single-precision sensitivity (dd\\_line)\n")
1572+
md.append(
1573+
"> Where reduced precision most moves the output — **typically the time integrator / "
1574+
"final accumulation, which is expected and benign**. This is *not* the same as where "
1575+
"cancellation originates; see **Catastrophic cancellation sites** below for that.\n"
1576+
)
15351577
_confirm_label = {True: "✅ confirmed", False: "⚠️ unconfirmed (suspect-only perturbation did not reproduce)", None: "— not checked"}
15361578
for r in cases_with_locs:
15371579
status = "❌ FAIL" if not r["passed"] else "✅ pass"
@@ -1558,6 +1600,8 @@ def _emit_github_summary(results: list, n_samples: int):
15581600
for line in snippet.splitlines():
15591601
md.append(f" {line}")
15601602
md.append(" ```")
1603+
if len(r["dd_line_locs"]) > 10:
1604+
md.append(f"- _…and {len(r['dd_line_locs']) - 10} more hotspot(s); see fp-stability-logs/_")
15611605
md.append("")
15621606

15631607
# dd_sym function names (collapsed, since less actionable than dd_line)
@@ -1571,12 +1615,19 @@ def _emit_github_summary(results: list, n_samples: int):
15711615
md.append(f"- `{sym}`")
15721616
md.append("\n</details>\n")
15731617

1574-
# Cancellation hotspots
1618+
# Cancellation hotspots — the ORIGIN view (where ill-conditioning concentrates).
15751619
cases_with_cancel = [r for r in results if r.get("cancellation_locs")]
15761620
if cases_with_cancel:
15771621
md.append("### Catastrophic cancellation sites\n")
1622+
md.append(
1623+
"> Where cancellation actually originates (subtraction of nearly-equal values). This is "
1624+
"the numerically interesting signal — and it usually differs from the sensitivity leader "
1625+
"above.\n"
1626+
)
15781627
for r in cases_with_cancel:
1579-
md.append(f"**`{r['name']}`** — {len(r['cancellation_locs'])} site(s)\n")
1628+
by_file = _cancellation_by_file(r["cancellation_locs"])
1629+
density = ", ".join(f"`{f}` ({n})" for f, n in by_file[:6])
1630+
md.append(f"**`{r['name']}`** — {len(r['cancellation_locs'])} site(s); concentrates in: {density}\n")
15801631
for fname, lineno in r["cancellation_locs"][:15]:
15811632
md.append(f"- `{fname}:{lineno}`")
15821633
snippet = _get_source_context(fname, lineno)
@@ -1585,6 +1636,8 @@ def _emit_github_summary(results: list, n_samples: int):
15851636
for line in snippet.splitlines():
15861637
md.append(f" {line}")
15871638
md.append(" ```")
1639+
if len(r["cancellation_locs"]) > 15:
1640+
md.append(f"- _…and {len(r['cancellation_locs']) - 15} more site(s); see fp-stability-logs/_")
15881641
md.append("")
15891642

15901643
# Float-max overflow sites
@@ -1595,6 +1648,8 @@ def _emit_github_summary(results: list, n_samples: int):
15951648
md.append(f"**`{r['name']}`** — {len(r['float_max_locs'])} site(s)\n")
15961649
for fname, lineno in r["float_max_locs"][:10]:
15971650
md.append(f"- `{fname}:{lineno}`")
1651+
if len(r["float_max_locs"]) > 10:
1652+
md.append(f"- _…and {len(r['float_max_locs']) - 10} more site(s); see fp-stability-logs/_")
15981653
md.append("")
15991654

16001655
with open(summary_path, "a") as f:

toolchain/mfc/test_fp_stability.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
from mfc.fp_stability import (
1010
_build_source_filter,
11+
_cancellation_by_file,
1112
_confirm_decision,
1213
_macro_context_in_lines,
1314
_mark_cancellation,
@@ -196,3 +197,24 @@ def test_mark_cancellation_false_for_different_basename():
196197
locs = [{"path": "m_foo.fpp", "start": 5, "end": 5}]
197198
_mark_cancellation(locs, [("m_bar.fpp", 5)])
198199
assert locs[0]["cancellation"] is False
200+
201+
202+
# --- cancellation-origin view: where cancellation concentrates ---
203+
204+
205+
def test_cancellation_by_file_counts_and_sorts_by_density():
206+
locs = [
207+
("src/simulation/m_weno.fpp", 10),
208+
("m_weno.fpp", 20),
209+
("a/m_riemann_solvers.fpp", 5),
210+
]
211+
assert _cancellation_by_file(locs) == [("m_weno.fpp", 2), ("m_riemann_solvers.fpp", 1)]
212+
213+
214+
def test_cancellation_by_file_breaks_ties_by_name():
215+
locs = [("z.fpp", 1), ("a.fpp", 2)]
216+
assert _cancellation_by_file(locs) == [("a.fpp", 1), ("z.fpp", 1)]
217+
218+
219+
def test_cancellation_by_file_empty():
220+
assert _cancellation_by_file([]) == []

0 commit comments

Comments
 (0)