Skip to content

Commit 54efaea

Browse files
igerberclaude
andcommitted
Address PR #453 R2 review (1 P3, isolate heavy-rejection test branch)
P3 — `test_enumerate_vertices_warns_on_heavy_rejection` previously used a fixture (X_tilde with 2 unique row types, 5 moments → all 3-row bases singular) where the assertion could pass via the `exhausted` warning instead of the intended `heavily constrained` branch. The branch was effectively untested. Rewrite the fixture: 5 moments × 1 nuisance column, C(5,2)=10 bases. By design, 6 bases trip LinAlgError (pairs among the singular-X_tilde indices) and 4 bases produce feasible vertices (each pairs a positive X_tilde with the unique negative X_tilde at index 4). 60% rejection rate hits the heavily-constrained branch specifically, not exhaustion. Switched to `pytest.warns(RuntimeWarning, match="heavily constrained")` so the test now fails if the message changes or if the wrong branch fires. Added a `len(vertices) >= 1` assertion to guard against the fixture inadvertently producing the exhausted-branch outcome. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7c0bdb2 commit 54efaea

1 file changed

Lines changed: 14 additions & 23 deletions

File tree

tests/test_methodology_honest_did.py

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -517,32 +517,23 @@ def test_enumerate_vertices_warns_on_exhausted_search(self):
517517
assert vertices == []
518518

519519
def test_enumerate_vertices_warns_on_heavy_rejection(self):
520-
"""Mixed-basis path: a partially-singular X_tilde produces some
521-
feasible vertices but rejects >= 50% of bases for LinAlgError.
522-
The warning helps users see that the recovered vertices came from
523-
a numerically fragile enumeration."""
520+
"""Mixed-basis path: 5 moments, 1 nuisance column. C(5, 2) = 10
521+
bases. By design, 6 bases hit LinAlgError (the singular pairs
522+
among indices {0,1,2,3} that share aligned nuisance/sigma values)
523+
and 4 bases produce feasible vertices (the (i, 4) pairs that pair
524+
a positive-X_tilde row with the unique negative-X_tilde row at
525+
index 4). 60% rejection rate trips the `heavily constrained`
526+
branch specifically, not the exhausted branch."""
524527
from diff_diff.honest_did import _enumerate_vertices
525528

526-
# 5 moments, 2 nuisance columns. C(5, 3) = 10 bases. Construct
527-
# X_tilde so that ~6 of 10 bases have rank-deficient A_sys.
528-
X_tilde = np.array([
529-
[1.0, 0.0],
530-
[1.0, 0.0],
531-
[1.0, 0.0],
532-
[0.0, 1.0],
533-
[0.0, 1.0],
534-
])
535-
sigma_tilde_diag = np.array([1.0, 1.0, 1.0, 1.0, 1.0])
536-
with warnings.catch_warnings(record=True) as caught:
537-
warnings.simplefilter("always", RuntimeWarning)
529+
X_tilde = np.array([[1.0], [1.0], [1.0], [2.0], [-1.0]])
530+
sigma_tilde_diag = np.array([1.0, 1.0, 1.0, 2.0, 1.0])
531+
with pytest.warns(RuntimeWarning, match="heavily constrained"):
538532
vertices = _enumerate_vertices(X_tilde, sigma_tilde_diag, n_moments=5)
539-
heavy = [w for w in caught if "heavily constrained" in str(w.message)]
540-
# If exhaustion fired instead, that's also a valid outcome — but the
541-
# construction is calibrated for the heavy-rejection branch
542-
exhausted = [w for w in caught if "exhausted" in str(w.message)]
543-
assert heavy or exhausted, (
544-
f"Expected heavily-constrained or exhausted warning; got "
545-
f"{[str(w.message) for w in caught]}, vertices={len(vertices)}"
533+
assert len(vertices) >= 1, (
534+
f"Heavy-rejection construction must still produce some feasible "
535+
f"vertices (otherwise the exhausted branch fires); got "
536+
f"{len(vertices)} vertices."
546537
)
547538

548539
def test_enumerate_vertices_quiet_on_healthy_enumeration(self):

0 commit comments

Comments
 (0)