Skip to content

Commit 22d7f65

Browse files
igerberclaude
andcommitted
Address PR #346 CI review round 14: P3 tighten NaN-contract wording
Round 14 is ✅ Looks good; this closes the final residual P3 for wording accuracy. **P3 (Docs): NaN-contract wording overstated** The prior wording said users can expect all five result fields to be finite or all NaN together. That is true on most degenerate paths (constant-y, no-dose-variation) but NOT on the single-cluster CR1 configuration of the mass-point path: `_fit_mass_point_2sls` returns `(att=beta_hat, se=nan)` there - `att` is finite because the Wald-IV ratio is well defined, while CR1 requires G >= 2 clusters so `se` is NaN. `safe_inference` then gates the downstream triple correctly, but the doc claim that `att` and `se` are coupled with the triple was inaccurate. Fix: rewrote the result-class docstring preamble and the REGISTRY Phase 2a NaN-propagation line to describe the invariant precisely: - GUARANTEED NaN coupling is on the downstream triple (`t_stat`, `p_value`, `conf_int`) via `safe_inference`. - `att` and `se` are RAW estimator outputs from the chosen fit path and are NOT gated by safe_inference. - On most degenerate paths, fit paths return `(nan, nan)` so all five move together, but on the single-cluster CR1 edge the fit path returns `(att=beta_hat, se=nan)` and only the downstream triple becomes NaN via the gate. No code behavior changes; the `assert_nan_inference` fixture already only checks the downstream triple, matching the restated contract. Targeted regression: 166 HAD tests (+1 sklearn-gated skip) + 545 total (+1 skipped) across Phase 1 and adjacent surfaces, all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2f6e1d0 commit 22d7f65

2 files changed

Lines changed: 21 additions & 8 deletions

File tree

diff_diff/had.py

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,26 @@ class HeterogeneousAdoptionDiDResults:
131131
``p_value``, and ``conf_int`` are routed through
132132
:func:`diff_diff.utils.safe_inference`, which returns NaN on all
133133
three whenever ``se`` is non-finite, zero, or negative. ``att`` and
134-
``se`` themselves are raw estimator outputs - when the estimator's
135-
fit path detects a degenerate configuration (constant outcome,
136-
no-variation-above-``d_lower``, divide-by-zero), it returns
137-
``(att=nan, se=nan)`` directly, which the safe-inference gate then
138-
propagates into the remaining three fields. Net effect: users can
139-
safely check ``np.isfinite(result.se)`` and expect all five fields
140-
to be finite or all NaN together on the current fit paths.
134+
``se`` themselves are RAW estimator outputs from the chosen fit
135+
path and are NOT gated by ``safe_inference``:
136+
137+
- On the degenerate fit configurations (constant outcome on the
138+
continuous paths, all-units-at-d_lower / no-dose-variation on the
139+
mass-point path), the fit path explicitly returns
140+
``(att=nan, se=nan)``, which combined with the safe-inference
141+
gate yields all five fields NaN together.
142+
- On the degenerate CR1 cluster configuration (mass-point path
143+
with a single cluster), ``_fit_mass_point_2sls`` returns
144+
``(att=beta_hat, se=nan)`` - ``att`` stays finite because the
145+
Wald-IV ratio is well defined, but the cluster-robust SE is
146+
not, so ``se`` is NaN and the downstream triple becomes NaN
147+
via the safe-inference gate.
148+
149+
So the guaranteed NaN coupling is on the downstream triple
150+
(``t_stat``, ``p_value``, ``conf_int``), not on ``att``. The
151+
``assert_nan_inference`` fixture in ``tests/conftest.py`` checks
152+
the downstream triple against the gate contract and does not
153+
assume ``att`` is NaN.
141154
142155
Attributes
143156
----------

docs/methodology/REGISTRY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2319,7 +2319,7 @@ Shipped as `did_had_pretest_workflow()` and surfaced via `practitioner_next_step
23192319
- [x] Phase 2a: `HeterogeneousAdoptionDiD` class with separate code paths for Design 1' (`continuous_at_zero`), Design 1 continuous-near-`` (`continuous_near_d_lower`), and Design 1 mass-point. Continuous paths compose Phase 1c's `bias_corrected_local_linear` and form the beta-scale WAS estimate `β̂ = (mean(ΔY) - τ̂_bc) / den` where `τ̂_bc` is the bias-corrected local-linear estimate of the boundary limit `lim_{d↓d̲} E[ΔY | D_2 ≤ d]` and `den = E[D_2]` for Design 1' (paper Theorem 1 / Equation 3 identification; Equation 7 sample estimator) or `den = E[D_2 - d̲]` for Design 1 (paper Theorem 3 / Equation 11, `WAS_{d̲}` under Assumption 6). Mass-point path uses a sample-average 2SLS estimator with instrument `1{D_{g,2} > d̲}` (paper Section 3.2.4).
23202320
- [x] Phase 2a: `design="auto"` detection rule (`min_g D_{g,2} < 0.01 · median_g D_{g,2}` → continuous_at_zero; modal-min fraction > 2% → mass_point; else continuous_near_lower). Implemented as strict first-match in `diff_diff.had._detect_design`; when `d.min() == 0` exactly, resolves `continuous_at_zero` unconditionally (modal-min check runs only when `d.min() > 0`). Edge case covered: 3% at `D=0` + 97% `Uniform(0.5, 1)` resolves to `continuous_at_zero`, matching the paper-endorsed Design 1' handling of small-share-of-treated samples.
23212321
- [x] Phase 2a: Panel validator (`diff_diff.had._validate_had_panel`) verifies `D_{g,1} = 0` for all units, rejects negative post-period doses (`D_{g,2} < 0`) front-door on the original (unshifted) scale, rejects `>2` time periods (staggered reduction queued for Phase 2b), and rejects unbalanced panels and NaN in outcome/dose/unit columns. Both Design 1 paths (`continuous_near_d_lower` and `mass_point`) additionally require `d_lower == float(d.min())` within float tolerance; mismatched overrides raise with a pointer to the unsupported (LATE-like / off-support) estimand.
2322-
- [x] Phase 2a: NaN-propagation tests across all 5 inference fields (`att`, `se`, `t_stat`, `p_value`, `conf_int`) covering constant-y and degenerate mass-point inputs. The inner `safe_inference()` gate covers the downstream triple (`t_stat`, `p_value`, `conf_int`); `att` and `se` are set to `NaN` by the fit paths themselves when they detect a degenerate configuration (constant outcome, no-variation-above-`d_lower`, divide-by-zero), so the five fields move to `NaN` together and the `assert_nan_inference` fixture holds.
2322+
- [x] Phase 2a: NaN-propagation tests covering constant-y, degenerate-mass-point, and single-cluster-CR1 inputs. The guaranteed NaN coupling is on the DOWNSTREAM triple (`t_stat`, `p_value`, `conf_int`) via the `safe_inference()` gate, which returns NaN on all three whenever `se` is non-finite, zero, or negative. `att` and `se` themselves are raw estimator outputs: on constant-y / no-dose-variation / divide-by-zero the fit paths return `(att=nan, se=nan)` so all five fields move to NaN together; on the degenerate single-cluster CR1 configuration on the mass-point path, `_fit_mass_point_2sls` returns `(att=beta_hat, se=nan)` - `att` is finite (Wald-IV is well defined) while `se` is NaN, so the downstream triple is NaN while `att` remains the raw 2SLS coefficient. The `assert_nan_inference` fixture in `tests/conftest.py` checks the downstream triple against this contract without requiring `att` to be NaN.
23232323
- **Note (mass-point SE):** Standard errors on the mass-point path use the structural-residual 2SLS sandwich `[Z'X]^{-1} · Ω · [Z'X]^{-T}` with `Ω` built from the structural residuals `u = ΔY - α̂ - β̂·D` (not the reduced-form residuals from an OLS-on-indicator shortcut). Supported: `classical`, `hc1`, and CR1 (cluster-robust) when `cluster=` is supplied. `hc2` and `hc2_bm` raise `NotImplementedError` pending a 2SLS-specific leverage derivation (the OLS leverage `x_i' (X'X)^{-1} x_i` is wrong for 2SLS; the correct finite-sample correction depends on `(Z'X)^{-1}` rather than `(X'X)^{-1}`) plus a dedicated R parity anchor. Queued for the follow-up PR.
23242324
- **Note (Design 1 identification):** `continuous_near_d_lower` and `mass_point` fits emit a `UserWarning` surfacing that `WAS_{d̲}` identification requires Assumption 6 (or Assumption 5 for sign identification only) beyond parallel trends, and that neither is testable via pre-trends. `continuous_at_zero` (Design 1', Assumption 3 only) does not emit this warning.
23252325
- **Note (CI endpoints):** Because the continuous-path `att` is `(mean(ΔY) - τ̂_bc) / den`, the beta-scale CI endpoints reverse relative to the Phase 1c boundary-limit CI: `CI_lower(β̂) = (mean(ΔY) - CI_upper(τ̂_bc)) / den` and `CI_upper(β̂) = (mean(ΔY) - CI_lower(τ̂_bc)) / den`. The `HeterogeneousAdoptionDiD.fit()` implementation computes `att ± z · se` directly via `safe_inference`, which handles the reversal naturally from the transformed point estimate.

0 commit comments

Comments
 (0)