Skip to content

feat: RegressionDiscontinuity - sharp RD estimator with rdrobust 4.0.0 parity (RDD PR-2)#684

Merged
igerber merged 3 commits into
mainfrom
rdd-phase2
Jul 13, 2026
Merged

feat: RegressionDiscontinuity - sharp RD estimator with rdrobust 4.0.0 parity (RDD PR-2)#684
igerber merged 3 commits into
mainfrom
rdd-phase2

Conversation

@igerber

@igerber igerber commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds RegressionDiscontinuity (alias RDD): sharp regression discontinuity estimation with robust bias-corrected inference, parity-targeting R rdrobust 4.0.0 end-to-end (PR-2 of the 2-PR RDD plan; PR-1 was the bandwidth-selection port, feat: rdrobust 4.0.0 sharp-RD bandwidth-selection port + R golden parity (RDD PR-1) #680).
  • Estimation-path port rdrobust_fit_sharp in diff_diff/_rdrobust_port.py: Q_q bias-correction score matrix, conventional/robust nearest-neighbor sandwich variances sharing same-side NN residuals, per-side fail-closed identification guards (distinct-support rank condition per window).
  • Public estimator: p/q orders on R's exact 0:20 surface (incl. p=0 local-constant), all 10 bwselect selectors, manual h/b/rho with R-exact resolution semantics, masspoints adjust/check/off, N<20 full-range fallback, warn-and-drop NaN handling.
  • Canonical binding: att/se/t_stat/p_value/conf_int are ONE coherent row (the robust bias-corrected row); rdrobust's printed headline is exposed as att_conventional with a full inference row; summary() prints the familiar three-row table. Full config echoes on the results object.
  • Docs sweep: REGISTRY.md RegressionDiscontinuity section (deviations labeled), RTD API page, references, llms guides, README/choosing-estimator one-liners, doc-deps entries, CHANGELOG.

Methodology references (required if estimator / math changes)

  • Method name(s): Sharp regression discontinuity with robust bias-corrected (RBC) inference; MSE/CER-optimal bandwidth selection.
  • Paper / source link(s): Calonico, Cattaneo & Titiunik (2014), Econometrica 82(6), doi:10.3982/ECTA11757; Calonico, Cattaneo, Farrell & Titiunik (2017), Stata Journal 17(2); Calonico, Cattaneo & Farrell (2018), JASA 113(522); Calonico, Cattaneo, Farrell & Titiunik (2019), REStat 101(3). Parity pin: CRAN rdrobust 4.0.0 source tarball (sha256 in diff_diff/_rdrobust_port.py). Paper reviews on file in docs/methodology/papers/calonico-*.md (docs: add RDD paper reviews (CCT 2014, CCFT 2017, CCF 2018, CCFT 2019) #675).
  • Any intentional deviations from the source (and why): each labeled in docs/methodology/REGISTRY.md (RegressionDiscontinuity section) - canonical att = tau_bc robust-row binding (uniform field semantics across the library; rdrobust prints tau_cl as headline), warn-instead-of-silent NaN drops and b-without-h handling, fail-closed targeted errors on degenerate designs (R propagates NaN/opaque errors or silent pseudo-inverse fits), v1 scope seams (fuzzy/covariates/cluster/weights/kink/diagnostics deferred).

Validation

  • Tests added/updated: tests/test_rdd.py (API/validation/echoes), tests/test_rdd_parity.py (16-config R golden parity at rtol=1e-9 incl. asymmetric selectors + 10-selector routing locks + Senate anchors incl. published 2017 Stata Journal numbers), tests/test_rdd_methodology.py (CCT 2014 Remark 7 identities across kernels and at p=0, invariances, NaN/degenerate fail-closed contracts), tests/test_rdrobust_port.py (port-level fit validation).
  • Backtest / simulation / notebook evidence (if applicable): R golden fixtures regenerated from pinned CRAN rdrobust 4.0.0 (benchmarks/R/generate_rdrobust_estimates_golden.R -> benchmarks/data/rdrobust_estimates_golden.json); worst observed parity deviation ~5e-11.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

https://claude.ai/code/session_01QGca52n6H8oDDXALjjrsp4

@github-actions

Copy link
Copy Markdown

Overall Assessment

⚠️ Needs changes — one unmitigated P1 methodology/assumption-check issue.

Executive Summary

  • The core robust-row inference binding uses safe_inference() consistently and is documented in REGISTRY.md; not a defect.
  • The sharp RD estimator broadly follows the documented rdrobust 4.0.0 parity target.
  • P1: manual-bandwidth fits bypass mass-point checking, so masspoints="check" can silently emit estimates on discrete running-variable data with no warning.
  • v1 exclusions for fuzzy RD, covariates, cluster variance, weights, kink, and diagnostics are documented as registry notes; informational only.
  • I could not run tests here: pytest and numpy are unavailable in the environment.

Methodology

Finding 1 — P1: masspoints="check" is ignored when h is supplied

Location: diff_diff/rdd.py:L551-L593, diff_diff/_rdrobust_port.py:L527-L552, docs/methodology/REGISTRY.md:L3630-L3636

Impact: CCT 2014 Remark 1 treats mass points / discrete running variables as an assumption risk requiring detection/warning. The registry says masspoints="check" warns and suggests adjust, but the public estimator only enters rdbwselect_sharp() on the data-driven bandwidth path. With manual h, the code skips the selector entirely, so masspoints="check" and masspoints="adjust" do not run the mass-point warning/check logic. This can silently report RD estimates on discrete running-variable data despite the user explicitly requesting the check.

Concrete fix: Move mass-point detection into RegressionDiscontinuity.fit() before the bandwidth-resolution branch, or extract a shared helper from rdbwselect_sharp(). Emit the same warnings for masspoints in {"check", "adjust"} regardless of whether h is manual. Add a test with rounded/tied x, h=..., and masspoints="check" asserting the warning fires.

Code Quality

Finding 2 — P2: bools are accepted for integer knobs

Location: diff_diff/rdd.py:L420-L449

Impact: p=True, nnmatch=True, or bwcheck=True pass validation because bool is an int subclass. That can silently change polynomial order or NN variance behavior from an invalid user input.

Concrete fix: Reject bool / np.bool_ for integer parameters, matching the existing _is_real_scalar() bool exclusion used for scalar bandwidth knobs.

Performance

No performance blockers found. The added estimator remains small-matrix WLS plus 1D NN scans; bandwidth selection is the expected dominant cost.

Maintainability

No blocking maintainability issues found. The port is heavily documented and line-mapped to rdrobust source behavior.

Tech Debt

P3 Informational: v1 RD scope seams are documented

Impact: Fuzzy RD, covariates, weights, cluster-robust variance, kink estimands, and rdplot/density diagnostics are omitted, but these are explicitly labeled in REGISTRY.md and API docs.

Concrete fix: None required for this PR.

Security

No secrets or security-sensitive additions found in the changed code or fixtures.

Documentation / Tests

Finding 3 — P2: missing test for manual-bandwidth mass-point check

Location: tests/test_rdd_methodology.py:L60-L72, tests/test_rdd.py:L213-L224

Impact: Existing tests cover mass-point warnings on the data-driven path and manual bandwidth semantics separately, but not their interaction. That is why the P1 above can slip through.

Concrete fix: Add RegressionDiscontinuity(h=..., masspoints="check") on tied/rounded x and assert UserWarning: Mass points detected.

Path to Approval

  1. Add mass-point detection/warning before the manual-vs-data-driven bandwidth branch in RegressionDiscontinuity.fit().
  2. Add a regression test for h=... plus masspoints="check" on tied running-variable data.
  3. Reject bool values for p, q, nnmatch, and bwcheck, or document intentional bool coercion if that is genuinely desired.

@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 2c45de650319c1e73b2d99b9a880776de82ca97f


Overall Assessment

Looks good — no unmitigated P0/P1 findings found in the re-review scope.

Executive Summary

  • Prior P1 is resolved: manual h fits now run mass-point detection before bandwidth resolution, with regression tests.
  • Prior bool/int validation issue is resolved for the public estimator constructor.
  • RDD canonical robust-row binding is documented in REGISTRY.md and implemented through safe_inference(), so it is not a defect.
  • v1 exclusions such as fuzzy RD, covariates, weights, cluster variance, kink, and diagnostics are documented seams.
  • I could not run tests: pytest is unavailable, and a direct Python smoke test failed because numpy is not installed.

Methodology

Finding 1 — P3 Informational: Documented rdrobust output-binding deviation

Severity: P3
Impact: att/se/t_stat/p_value/conf_int bind to the robust bias-corrected row, while rdrobust prints the conventional coefficient as the headline. This is explicitly documented as a deviation and preserves diff-diff’s coherent inference-field contract.
Concrete fix: None required. See docs/methodology/REGISTRY.md:L3541-L3552 and implementation at diff_diff/rdd.py:L661-L682.

Finding 2 — P3 Informational: Documented v1 RD scope seams

Severity: P3
Impact: Fuzzy RD, covariates, weights, cluster-robust RD variance, kink estimands, and diagnostics are not shipped in this PR. These are explicitly documented follow-ups, not silent unsupported paths.
Concrete fix: None required. See docs/methodology/REGISTRY.md:L3601-L3608 and docs/api/regression_discontinuity.rst:L24-L32.

Prior P1 check: masspoints="check" with manual h is now handled at diff_diff/rdd.py:L549-L573, before the manual-vs-data-driven bandwidth branch, and covered by tests/test_rdd_methodology.py:L78-L98.

Code Quality

No blocking findings. Public integer knobs now reject bools via _is_int_scalar() and constructor validation at diff_diff/rdd.py:L415-L452.

Performance

No findings. The implementation remains small weighted least-squares fits plus 1D nearest-neighbor scans; bandwidth selection is the expected dominant cost.

Maintainability

No findings. The methodology-sensitive choices are heavily documented in code and registry notes, including Q/q score construction and robust/conventional sandwich assembly at diff_diff/_rdrobust_port.py:L978-L999.

Tech Debt

Finding 3 — P3 Informational: Deferred RD feature scope is documented

Severity: P3
Impact: Deferred RD functionality is visible in user docs and methodology registry, so users are not silently routed into unsupported methods.
Concrete fix: None required.

Security

No findings. Static scan of changed files did not identify secrets or credential material.

Documentation / Tests

No blocking findings. The PR adds parity and methodology coverage, including full golden parity at tests/test_rdd_parity.py:L75-L116, Remark 7 anchors at tests/test_rdd_methodology.py:L20-L58, and NaN/degenerate contracts at tests/test_rdd_methodology.py:L143-L213.

Tests were not executed in this environment because pytest and numpy are unavailable.

@igerber igerber added ready-for-ci Triggers CI test workflows and removed ready-for-ci Triggers CI test workflows labels Jul 13, 2026
igerber and others added 3 commits July 13, 2026 09:54
Estimation-path port + public estimator + docs sweep (PR-2 of 2).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QGca52n6H8oDDXALjjrsp4
…ols on integer knobs

rdrobust.R:365-380 runs the mass-point check in rdrobust() itself, ahead of
the manual-vs-data-driven bandwidth split, so the warning must fire on
manual-h fits too (verified on installed 4.0.0). fit() now performs the
detection and the port's rdbwselect-level warning is suppressed on the
estimator path to mirror R's single warning per fit. Integer knobs
(p/q/nnmatch/bwcheck) now reject bool (int subclass) inputs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QGca52n6H8oDDXALjjrsp4
…job)

The type: ignore[assignment] comments on the beta_p_* dataclass fields are
unused under the pinned CI mypy config, which now fails on unused ignores
(warn_unused_ignores = true).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QGca52n6H8oDDXALjjrsp4
@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 86b7c28a357b7c944ffcc92ee7202813678e2a72


Overall Assessment

Looks good — no unmitigated P0/P1 findings found in the re-review scope.

Executive Summary

  • Prior P1 is addressed: mass-point detection now runs before manual/data-driven bandwidth branching, including manual h fits.
  • Prior public bool/int validation issue is addressed via _is_int_scalar() and constructor tests.
  • RDD methodology-sensitive choices are documented in docs/methodology/REGISTRY.md and implemented consistently with those notes.
  • Canonical att binding to the robust bias-corrected row is a documented deviation, so not a defect.
  • I could not run tests locally because numpy, pandas, scipy, and pytest are unavailable.

Methodology

Finding 1 — P3 Informational: Documented rdrobust output-binding deviation

Severity: P3
Impact: att/se/t_stat/p_value/conf_int bind to the robust bias-corrected row, while rdrobust prints the conventional estimate as its headline. This is explicitly documented and preserves the library-wide coherent inference contract.
Concrete fix: None required. See docs/methodology/REGISTRY.md:L3541-L3552 and implementation at diff_diff/rdd.py:L662-L684.

Finding 2 — P3 Informational: Documented v1 scope seams

Severity: P3
Impact: Fuzzy RD, covariates, weights, cluster variance, kink estimands, and diagnostics are excluded from v1, but this is documented rather than silently exposed.
Concrete fix: None required. See docs/methodology/REGISTRY.md:L3601-L3608 and docs/api/regression_discontinuity.rst:L24-L32.

Prior P1 checks are resolved: mass-point detection is performed at diff_diff/rdd.py:L550-L574 before the manual bandwidth branch, with coverage at tests/test_rdd_methodology.py:L78-L98. Robust/conventional/RBC inference rows use safe_inference() at diff_diff/rdd.py:L662-L667.

Code Quality

No blocking findings. Public integer knobs reject bools via _is_int_scalar() at diff_diff/rdd.py:L416-L419, with constructor validation at diff_diff/rdd.py:L426-L453 and tests at tests/test_rdd.py:L83-L92.

Performance

No findings. The implementation uses expected local WLS and same-side nearest-neighbor residual scans; bandwidth selection remains the dominant cost.

Maintainability

No findings. The Q/q bias-correction score matrix and sandwich construction are localized and documented at diff_diff/_rdrobust_port.py:L978-L999, with registry notes tying deviations to source behavior.

Tech Debt

Finding 3 — P3 Informational: Deferred RD features are documented

Severity: P3
Impact: Deferred RD features are visible in API docs and methodology registry, so users are not silently routed into unsupported surfaces.
Concrete fix: None required.

Security

No findings in the changed RDD implementation, fixtures, or docs.

Documentation / Tests

No blocking findings. The PR adds parity, methodology, and validation tests, including R golden parity at tests/test_rdd_parity.py:L76-L119, Remark 7 methodology anchors at tests/test_rdd_methodology.py:L22-L54, and degenerate/NaN contracts at tests/test_rdd_methodology.py:L139-L213.

Tests were not executed in this environment: python -m pytest ... failed because pytest is unavailable, and numpy/pandas/scipy are also missing.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 13, 2026
@igerber igerber merged commit 90b524b into main Jul 13, 2026
38 of 39 checks passed
@igerber igerber deleted the rdd-phase2 branch July 13, 2026 15:51
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