Skip to content

Commit 2250c67

Browse files
igerberclaude
andcommitted
Address CI review P3: broaden B-spline derivative warning scope
CI reviewer correctly noted that dPsi feeds both ACRT point estimates (continuous_did.py:1026-1046, bootstrap path at 1524-1561) and the analytical/bootstrap inference, not just inference alone. Updated the warning text and the REGISTRY note to say both outputs are biased when derivative construction fails. No behavior change; text-only accuracy fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bac7a2b commit 2250c67

2 files changed

Lines changed: 5 additions & 4 deletions

File tree

diff_diff/continuous_did_bspline.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,10 @@ def bspline_derivative_design_matrix(x, knots, degree, include_intercept=True):
172172
f"(indices {failed_basis_indices}); their derivative columns "
173173
f"are zero. This typically indicates a malformed knot vector "
174174
f"(too few knots for the chosen degree, non-monotonic, or "
175-
f"repeated interior knots). ContinuousDiD inference may be "
176-
f"biased; consider increasing the number of distinct doses "
177-
f"or reducing the B-spline degree.",
175+
f"repeated interior knots). Both ACRT point estimates and "
176+
f"analytical/bootstrap inference depend on this derivative "
177+
f"matrix, so both may be biased. Consider increasing the "
178+
f"number of distinct doses or reducing the B-spline degree.",
178179
UserWarning,
179180
stacklevel=2,
180181
)

docs/methodology/REGISTRY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ See `docs/methodology/continuous-did.md` Section 4 for full details.
723723
not-yet-treated controls. When `anticipation=0` (default), behavior is
724724
unchanged.
725725
- **Boundary knots**: Knots are built once from all treated doses (global, not per-cell) to ensure a common basis across (g,t) cells for aggregation. Evaluation grid is clamped to training-dose boundary knots (`range(dose)`). R's `contdid` v0.1.0 has an inconsistency where `splines2::bSpline(dvals)` uses `range(dvals)` instead of `range(dose)`, which can produce extrapolation artifacts at dose grid extremes. Our approach avoids extrapolation and is methodologically sound.
726-
- **Note:** `bspline_derivative_design_matrix` previously swallowed `ValueError` from `scipy.interpolate.BSpline` in the per-basis derivative loop, leaving affected columns of the derivative design matrix as zero with no user-facing signal. It now aggregates the failed basis indices and emits ONE `UserWarning` naming them so downstream ContinuousDiD inference doesn't silently use a biased `dPsi`. The all-identical-knot degenerate case (single dose value) remains silently handled — derivatives there are mathematically zero. Axis-C finding #12 in the Phase 2 silent-failures audit.
726+
- **Note:** `bspline_derivative_design_matrix` previously swallowed `ValueError` from `scipy.interpolate.BSpline` in the per-basis derivative loop, leaving affected columns of the derivative design matrix as zero with no user-facing signal. It now aggregates the failed basis indices and emits ONE `UserWarning` naming them. Both ACRT point estimates and analytical/bootstrap inference read the same `dPsi` matrix (see `continuous_did.py:1026-1046` and the bootstrap ACRT path at `continuous_did.py:1524-1561`), so both are biased on a partial derivative-construction failure — the warning wording makes that explicit. The all-identical-knot degenerate case (single dose value) remains silently handled — derivatives there are mathematically zero. Axis-C finding #12 in the Phase 2 silent-failures audit.
727727

728728
### Implementation Checklist
729729

0 commit comments

Comments
 (0)