Skip to content

Commit a307656

Browse files
igerberclaude
andcommitted
bench(refresh): SDID weight gate requires id alignment - no positional fallback
The registry documents the SDID weight-parity gate as id-aligned and fail-closed, but compare_weight_vectors() silently fell back to positional comparison when ids were absent, and the runner never checked aligned_by_ids - a benchmark-script regression that stopped emitting ids would have quietly weakened the documented contract. - compare_weight_vectors(require_ids=True): missing ids on either side now FAILS the comparison; the SDID publication gate passes it. Positional fallback remains available (require_ids=False) for legacy artifacts only. - ensure_mpdta() integrity checks use explicit raises instead of assert (asserts vanish under python -O). - TODO row tracks the intentionally deferred phase-2 artifact/prose push (removed by that push itself). - Tests: 48 total (missing-ids-fail-closed with positionally identical weights, valid-ids pass, non-required fallback preserved). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GPX5Rv8ozQXPdUV23QTfjr
1 parent 65b5d66 commit a307656

4 files changed

Lines changed: 43 additions & 6 deletions

File tree

TODO.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m
4545

4646
### Testing / docs
4747

48+
| Issue | Location | Origin | Effort | Priority |
49+
|-------|----------|--------|--------|----------|
50+
| Benchmark refresh phase 2 (same PR #672): run the gated timed refresh on an idle machine, commit `benchmarks/refresh_2026_07/results/refresh_results.json`, regenerate the marker-bounded regions of `docs/benchmarks.rst` via `gen_benchmark_tables.py`, and reconcile the remaining pre-refresh prose (protocol bullets "3 replications / mean ± std", combined BasicDiD/TWFE wording, SDID note under the perf table, Key Observations, "Reproducing Benchmarks" section, `llms.txt` speedup cross-references). Row removed by the phase-2 push itself. | `benchmarks/refresh_2026_07/`, `docs/benchmarks.rst` | #672 | Mid | Medium |
51+
4852
| Issue | Location | Origin | Effort | Priority |
4953
|-------|----------|--------|--------|----------|
5054

benchmarks/refresh_2026_07/refresh_common.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,18 @@ def compare_weight_vectors(
131131
atol: float = 1e-8,
132132
py_ids: Any = None,
133133
r_ids: Any = None,
134+
require_ids: bool = False,
134135
) -> Dict[str, Any]:
135136
"""
136137
Compare SDID unit/time weight vectors, aligning by unit/period id when
137138
both sides provide ids (ordering-robust: Python's
138139
get_unit_weights_df() sorts by descending weight, R emits panel order).
139-
Falls back to positional comparison when ids are absent. Enforces the
140-
docs' identical-weights claim with an auditable committed metric;
141-
fail-closed on length/key mismatch, duplicates, or non-finite entries.
140+
With require_ids=True (the SDID publication gate), missing ids on
141+
either side FAILS the comparison - the documented id-alignment contract
142+
can never silently degrade to positional order if a benchmark script
143+
regresses. Without it, positional comparison is a permitted fallback
144+
for legacy artifacts. Fail-closed on length/key mismatch, duplicates,
145+
or non-finite entries; metrics are committed for audit.
142146
"""
143147
py = list(py_weights or [])
144148
r = list(r_weights or [])
@@ -149,6 +153,8 @@ def compare_weight_vectors(
149153
"aligned_by_ids": False,
150154
"ok": False,
151155
}
156+
if require_ids and (py_ids is None or r_ids is None):
157+
return metrics
152158
if not py or len(py) != len(r):
153159
return metrics
154160

@@ -847,9 +853,12 @@ def ensure_mpdta() -> Path:
847853
"first.treat": "first_treat",
848854
}
849855
)[["unit", "time", "outcome", "first_treat"]]
850-
assert len(df) == 2500, f"MPDTA rows: {len(df)} != 2500"
851-
assert df["unit"].nunique() == 500, "MPDTA counties != 500"
852-
assert set(df["first_treat"].unique()) == {0, 2004, 2006, 2007}, "MPDTA cohorts unexpected"
856+
if len(df) != 2500:
857+
raise RuntimeError(f"MPDTA rows: {len(df)} != 2500")
858+
if df["unit"].nunique() != 500:
859+
raise RuntimeError("MPDTA counties != 500")
860+
if set(df["first_treat"].unique()) != {0, 2004, 2006, 2007}:
861+
raise RuntimeError("MPDTA cohorts unexpected")
853862
save_benchmark_data(df, path)
854863
return path
855864

benchmarks/refresh_2026_07/run_refresh.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,9 @@ def _att(arm: str) -> float:
368368
atol=1e-8,
369369
py_ids=py_res.get(id_field),
370370
r_ids=r_res.get(id_field),
371+
# Documented contract: the publication gate is id-aligned
372+
# and may never silently degrade to positional order.
373+
require_ids=True,
371374
)
372375
weight_detail[f"{surface}:{py_arm}"] = metrics
373376
if not metrics["ok"]:

tests/test_benchmark_refresh_gates.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,27 @@ def test_string_ids_from_r_align_with_int_ids(self):
428428
m = rc.compare_weight_vectors([0.6, 0.4], [0.4, 0.6], py_ids=[10, 3], r_ids=["3", "10"])
429429
assert m["aligned_by_ids"] and m["ok"]
430430

431+
def test_missing_ids_fail_closed_when_required(self):
432+
# The SDID publication gate documents id alignment; positionally
433+
# identical weights must still FAIL if either side stops emitting
434+
# ids (a script regression cannot silently weaken the contract).
435+
m = rc.compare_weight_vectors([0.5, 0.5], [0.5, 0.5], require_ids=True)
436+
assert not m["ok"] and not m["aligned_by_ids"]
437+
m2 = rc.compare_weight_vectors(
438+
[0.5, 0.5], [0.5, 0.5], py_ids=[1, 2], r_ids=None, require_ids=True
439+
)
440+
assert not m2["ok"]
441+
442+
def test_require_ids_passes_with_valid_ids(self):
443+
m = rc.compare_weight_vectors(
444+
[0.5, 0.5], [0.5, 0.5], py_ids=[1, 2], r_ids=[1, 2], require_ids=True
445+
)
446+
assert m["ok"] and m["aligned_by_ids"]
447+
448+
def test_positional_fallback_still_allowed_when_not_required(self):
449+
m = rc.compare_weight_vectors([0.5, 0.5], [0.5, 0.5])
450+
assert m["ok"] and not m["aligned_by_ids"]
451+
431452
def test_mismatched_id_sets_fail_closed(self):
432453
m = rc.compare_weight_vectors([0.5, 0.5], [0.5, 0.5], py_ids=[1, 2], r_ids=[1, 3])
433454
assert not m["ok"] and m["max_abs_diff"] is None

0 commit comments

Comments
 (0)