Skip to content

feat(continuous-did): discrete-treatment saturated regression (treatment_type="discrete")#618

Merged
igerber merged 4 commits into
mainfrom
feature/continuous-did-discrete
Jul 5, 2026
Merged

feat(continuous-did): discrete-treatment saturated regression (treatment_type="discrete")#618
igerber merged 4 commits into
mainfrom
feature/continuous-did-discrete

Conversation

@igerber

@igerber igerber commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add treatment_type="discrete" to ContinuousDiD — a saturated regression for multi-valued / discrete dose (CGBS 2024 Eq. 4.1). Each distinct dose level gets its own effect coefficient (ATT(d_j) = mean_{D=d_j}(ΔY) − control, a per-level 2×2 DiD) instead of a smoothed B-spline curve; ACRT(d_j) is a finite difference (forward at the lowest level, backward elsewhere) so ATT/ACRT share the J-point grid and ACRT^glob stays well-defined.
  • Implemented as an exact basis swap: the B-spline design/evaluation/derivative trio is replaced by an indicator/identity/finite-difference trio. Every downstream quantity is linear in beta, so the analytical-SE, multiplier-bootstrap, covariate (reg/dr), and survey machinery is reused unchanged and reduces analytically to the per-level 2×2 DiD standard error (bread @ psi_bar = ones(J); the common control mean cancels in ACRT via L·1 = 0). reg/dr share ACRT(d_j) point+SE on the intercept-free basis; only the ATT level differs.
  • Fail-closed edges: multi-cohort heterogeneous dose support → NotImplementedError (support-aware aggregation deferred; single-cohort / 2-period / shared-support multi-cohort supported); off-support dvalsValueError; a survey/subpopulation-zeroed dose level → ValueError, checked per (g,t) cell (not just globally) so a level zeroed for one cohort cannot silently drop to a zero-coefficient column.
  • Default treatment_type="continuous" (B-spline) path is numerically unchanged.

Methodology references

  • Method name(s): ContinuousDiD discrete-treatment saturated regression (treatment_type="discrete")
  • Paper / source link(s): Callaway, Goodman-Bacon & Sant'Anna (2024), "Difference-in-Differences with a Continuous Treatment" (NBER WP 32117), §4.1 / Eq. 4.1. Documented in docs/methodology/REGISTRY.md § ContinuousDiD (Note Add multi-period DiD support #6, Key Equations) and docs/methodology/continuous-did.md §5.1.
  • Any intentional deviations from the source (and why): Library extension — R contdid v0.1.0 accepts treatment_type in its signature but does not implement the discrete path, so there is no external R anchor. Validation is R-free: exact hand-calc of ATT(d_j)/ACRT/overall_att and the analytical SE vs a direct per-level 2×2 reconstruction, DGP recovery, and MC coverage (analytical + bootstrap). The forward-difference ACRT convention at the lowest dose is a documented boundary choice (no paper/R anchor there), recorded in REGISTRY Note Add multi-period DiD support #6.

Validation

  • Tests added/updated: tests/test_methodology_continuous_did.py (TestDiscreteSaturated: hand-calc ATT/ACRT/overall + analytical SE, ACRT boundary, DGP recovery, staggered shared-support, MC coverage @slow) and tests/test_continuous_did.py (TestDiscreteSaturatedAPI: dr×discrete ACRT identity, survey×discrete weighted means, exactly-identified boundary, heterogeneous-support/off-support/survey-zero-level/per-cell guards, params round-trip, continuous default unchanged).
  • Backtest / simulation / notebook evidence (if applicable): MC coverage tests (analytical + multiplier bootstrap) confirm nominal coverage for ATT(d_j) and ACRT(d_j) under a discrete-dose DGP.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

🤖 Generated with Claude Code

https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi

igerber and others added 3 commits July 5, 2026 10:09
…ent_type="discrete")

Adds treatment_type="discrete" to ContinuousDiD for multi-valued / discrete
dose (CGBS 2024 Eq. 4.1). Each distinct dose level gets its own effect
coefficient - ATT(d_j) = mean_{D=d_j}(dY) - control, a per-level 2x2 DiD -
instead of a smoothed B-spline curve; ACRT(d_j) is a finite difference
(forward at the lowest level, backward elsewhere) so ATT/ACRT share the
J-point grid and ACRT^glob stays well-defined.

Implemented as an exact basis swap: the B-spline design/evaluation/derivative
trio is replaced by an indicator/identity/finite-difference trio, and every
downstream quantity is linear in beta, so the analytical-SE, multiplier-
bootstrap, covariate (reg/dr), and survey machinery is reused unchanged and
reduces analytically to the per-level 2x2 DiD standard error
(bread @ psi_bar = ones(J); the common control mean cancels in ACRT via
L.1 = 0). reg/dr share ACRT(d_j) point+SE on the intercept-free basis
(partition-of-unity residual-invariance + L.1 = 0); only the ATT level differs.

Fail-closed edges: multi-cohort heterogeneous dose support raises
NotImplementedError (support-aware aggregation deferred); an off-support dvals
value raises ValueError; a survey-zeroed dose level raises ValueError. The
default treatment_type="continuous" (B-spline) path is numerically unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi
…il-closed)

Local review found a P1 blind spot: the fit-time survey-weight guard only
checks global positive dose-level support. In a shared-support multi-cohort
discrete fit, survey/subpopulation weights can zero out a dose level for one
cohort while another keeps it positive — the global guard passes, but that
cohort's (g,t) cell then has an all-zero saturated column that solve_ols drops
and beta_pred zeroes, silently biasing ATT(d)/ACRT(d) aggregation.

Add a per-cell support check in _compute_dose_response_gt: every dose level must
have positive effective treated mass in the cell, else raise ValueError before
OLS. Covers n_bootstrap>0 too (runs during the initial cell pass). Adds a
two-cohort survey test for the cell-level zero-support case and documents the
policy in REGISTRY Note #6.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi
… n_bootstrap

Lock the per-(g,t)-cell zero-support ValueError for both the analytical
(n_bootstrap=0) and bootstrap (n_bootstrap>0) survey paths (the guard runs in
the initial cell pass, before inference). Closes the optional test-hardening
item from local review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Overall Assessment

⚠️ Needs changes — one P1 methodology issue.

Executive Summary

  • The new treatment_type="discrete" saturated ATT path matches the documented per-level 2x2 DiD construction for J >= 2.
  • The forward-difference boundary convention is documented in REGISTRY.md, so I am not treating it as a defect.
  • The J == 1 discrete-dose case returns ACRT = 0, which conflicts with the project docs’ binary-treatment equivalence and the paper’s d0 = 0 discrete ACRT setup.
  • No new inline inference anti-patterns were found in the changed ContinuousDiD files.
  • I could not run tests locally because the environment cannot import numpy.

Methodology

Finding 1 — P1: Single positive dose level reports ACRT = 0

Location: diff_diff/continuous_did_bspline.py:L324-L368, diff_diff/continuous_did.py:L1484-L1488, docs/methodology/continuous-did.md:L186-L205

Impact: For treatment_type="discrete" with one positive dose level, the derivative matrix returns all zeros, so ACRT(d) and ACRT^glob are reported as zero. That is not the paper’s discrete setup: Eq. 4.1 uses the zero-dose group as omitted and notes d0 = 0, with discrete ACRT formed as adjacent-dose differences. The project docs also state that when D in {0,1}, ACRT = ATT. The current behavior therefore gives wrong ACRT output for binary / single-level discrete treatments. The warning says ACRT will be zero, so I am classifying this as P1 rather than P0. (arxiv.org)

Concrete fix: Do not return an all-zero derivative for J == 1. Either:

  • paper-consistent behavior: set the derivative row to [1 / d1], so ACRT(d1) = ATT(d1) / d1 and for d1 == 1, ACRT = ATT; or
  • fail closed with ValueError / NaN ACRT if the library’s documented forward-positive-level convention requires at least two positive levels.

Add a methodology test with a single positive dose d=1.0 and known effect tau, asserting ATT=tau, ACRT=tau, overall_acrt=tau, and corresponding SE/CI behavior.

Informational — P3: Documented deviations are not defects

Location: docs/methodology/REGISTRY.md:L944-L992, TODO.md:L32

Impact: The forward-difference convention at the lowest positive level for J >= 2, and the deferred heterogeneous-support multi-cohort aggregation, are documented in the registry/TODO. No action required under the review rules.

Concrete fix: None required.

Code Quality

No independent code quality findings. The basis swap is localized and uses shared downstream IF/bootstrap machinery.

Performance

No findings. The saturated indicator basis is small for the intended few-level discrete-dose case.

Maintainability

No independent findings. The new helper trio mirrors the B-spline helper shape, which keeps the dispatch readable.

Tech Debt

P3 informational only: heterogeneous-support discrete aggregation is explicitly tracked in TODO.md and documented as deferred. No blocking action required.

Security

No findings. I saw no secret material or new risky I/O paths in the diff.

Documentation/Tests

The P1 methodology issue needs a dedicated single-positive-dose test. Existing new tests cover J=3, shared-support staggered cohorts, survey zero-weight guards, off-support dvals, and parameter propagation, but not the binary / J == 1 discrete saturated path.

Path to Approval

  1. Change the J == 1 saturated derivative behavior so it no longer reports zero ACRT for a single positive dose.
  2. Add a regression test for treatment_type="discrete" with one positive dose level, including d=1.0, checking ACRT = ATT and overall_acrt = overall_att.
  3. If the chosen behavior is to reject J == 1 instead of estimating ACRT, document that explicitly in REGISTRY.md with a Note label and assert the fail-closed error in tests.

…ixes CI P1)

CI review flagged that a single positive dose (J=1) returned ACRT=0, which
contradicts the documented binary identity ACRT=ATT and the paper's Eq. 4.1
d_0=0 discrete setup. The prior forward-difference at the lowest dose was a
deviation; switch to the paper's backward difference on the grid {0, d_1, ...,
d_J} (d_0=0, ATT(0)=0), so ACRT(d_1) = ATT(d_1)/d_1 and a binary D in {0,1}
gives ACRT=ATT. Only the lowest dose's row changes; the j>=2 adjacent backward
differences are unchanged.

Consequence (correct): reg/dr share ACRT(d_j) for j>=2 (the constant DR
augmentation cancels in adjacent differences) but differ at ACRT(d_1) by
eta_cont/d_1 -- d_1 references the fixed baseline ATT(0)=0, which the
augmentation does not shift. The dr influence function now carries the
augmentation variance at d_1 (analytical ACRT(d_1) SE matches the multiplier
bootstrap); applied only on the discrete path, so the validated B-spline
covariate IF is byte-identical. The all-same-dose "ACRT will be 0" warning is
suppressed on the discrete path (single-dose ACRT = ATT(d_1)/d_1 != 0).

Updates REGISTRY Note #5/#6 + Key Equations, continuous-did.md 5.1, CHANGELOG,
llms-full.txt, docstring; adds binary-single-dose and dr-ACRT-analytical-vs-
bootstrap tests; updates the ACRT-boundary / reg-dr / DGP-recovery tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHDijzf8zHXk5T8ahS2mKi
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 1140ae8d166b3538de0d2d4c489676000a7eaaef


Overall Assessment

Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • Prior P1 is resolved: J == 1 discrete doses now use ACRT(d_1) = ATT(d_1)/d_1, with binary d_1=1 covered by a regression test.
  • The saturated indicator basis and backward finite-difference operator match the documented CGBS Eq. 4.1 interpretation in REGISTRY.md.
  • The DR covariate path now correctly treats the lowest-dose ACRT differently from adjacent positive-dose differences, and documents that behavior.
  • Heterogeneous multi-cohort support and over-parameterized per-level fits are documented in REGISTRY.md/TODO.md, so they are not blockers.
  • I could not run tests locally because pytest is not installed in this environment.

Methodology

P3 — Informational: documented discrete-path restrictions

Location: docs/methodology/REGISTRY.md:L945-L997, TODO.md:L32, diff_diff/continuous_did.py:L541-L601

Impact: Multi-cohort heterogeneous dose support raises NotImplementedError, and sparse saturated fits warn about degenerate per-level SEs. These are explicitly documented as library-extension/fail-closed choices.

Concrete fix: None required.

Code Quality

No findings. The basis dispatch is localized in diff_diff/continuous_did.py:L1457-L1471, and the saturated helper trio mirrors the existing B-spline helper shape in diff_diff/continuous_did_bspline.py:L243-L370.

Performance

No findings. The saturated basis is an indicator matrix over observed dose levels; no concerning new asymptotic cost is introduced for the intended few-level discrete-dose case.

Maintainability

No findings. treatment_type is propagated through validation, get_params(), results metadata, and summary output.

Tech Debt

P3 — Informational: heterogeneous-support aggregation deferred and tracked

Location: TODO.md:L32, docs/methodology/REGISTRY.md:L997

Impact: Support-aware aggregation for cohorts with different observed dose levels is deferred, but the implementation fails closed rather than silently biasing absent levels toward zero.

Concrete fix: None required for this PR.

Security

No findings. The diff does not add risky I/O, credential handling, or secret material.

Documentation/Tests

No blocking findings. The prior missing binary/single-dose test is now covered at tests/test_methodology_continuous_did.py:L1252-L1274, and DR/discrete ACRT behavior is covered at tests/test_continuous_did.py:L1807-L1838.

Verification note: targeted pytest execution was not possible because pytest is not installed.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 5, 2026
@igerber igerber merged commit 110f2b2 into main Jul 5, 2026
35 of 36 checks passed
@igerber igerber deleted the feature/continuous-did-discrete branch July 5, 2026 16:48
@igerber igerber mentioned this pull request Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant