Skip to content

synthetic control: PR-A paper reviews (ADH 2010/2015, Abadie 2021 JEL, CWZ 2021)#497

Merged
igerber merged 1 commit into
mainfrom
feature/synthetic-control-paper-reviews
May 29, 2026
Merged

synthetic control: PR-A paper reviews (ADH 2010/2015, Abadie 2021 JEL, CWZ 2021)#497
igerber merged 1 commit into
mainfrom
feature/synthetic-control-paper-reviews

Conversation

@igerber
Copy link
Copy Markdown
Owner

@igerber igerber commented May 29, 2026

Summary

  • First wave (PR-A) of a paper-reviews-first methodology promotion for the classic Synthetic Control Method (Abadie-Diamond-Hainmueller), distinct from the existing SyntheticDiD. Adds four methodology paper-review docs, each sourced strictly from its primary paper:
    • abadie-diamond-hainmueller-2010 — core estimator: V-matrix nested optimization (inner simplex QP + outer MSPE/CV V-selection), gap estimator, in-space placebo + post/pre-RMSPE-ratio permutation inference, Appendix B bias bound, edge cases.
    • abadie-diamond-hainmueller-2015 — diagnostics: in-time placebo, leave-one-out donor removal, cross-validation V-selection, regression-vs-SC extrapolation.
    • abadie-2021 (JEL) — practical guide: feasibility, data requirements, contextual failure modes, inference toolkit (RMSPE-ratio, CI by test inversion, one-sided), weight sparsity.
    • chernozhukov-wuthrich-zhu-2021 — conformal inference: S_q statistic, permutation p-value, i.i.d. + moving-block permutation schemes, Algorithm 1 pointwise CIs by test inversion.
  • Reviews only; no source or implementation changes. Implementation follows in subsequent PRs.

Methodology references

Validation

  • Tests added/updated: No test changes (paper reviews only; no code).
  • Backtest / simulation / notebook evidence (if applicable): N/A.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

🤖 Generated with Claude Code

@github-actions
Copy link
Copy Markdown

Overall Assessment

⚠️ Needs changes

No P0 silent-numerical bug stood out in the touched estimators, but the PR does introduce an unreviewed methodology claim on the Conley surface and reverts part of the public EfficientDiD contract.

Executive Summary

  • This is not a docs-only PR in practice; it changes EfficientDiD constructor/results behavior and Conley user-facing methodology messaging.
  • P1 methodology: the Conley weights / survey_design deferral is now described as a “Bertanha-Imbens 2014 weighted-Conley” follow-up even though the repo has no reviewed source entry for that method, and TODO.md itself says that source does not cover the explicit Conley case.
  • P2 code-quality: EfficientDiD.set_params() lost its rollback behavior and can now leave a partially-mutated estimator behind after a validation error.
  • P2 maintainability/API: EfficientDiD drops vcov_type, cluster_name / n_clusters, to_dict(), and summary variance metadata without a deprecation path; some docs were updated, others still describe the old common results surface.
  • P3 tech-debt/docs: the Conley methodology tracker is demoted back to “In Progress,” reopening review debt without a clear new defect or TODO-tracked rationale.

Methodology

  • Severity: P1. Impact: Affected method is ConleySpatialHAC, specifically the unsupported weighted / survey-aware variance surface. The new runtime and docs now present that boundary as a “Bertanha-Imbens 2014 weighted-Conley” roadmap, but there is no corresponding paper review in docs/methodology/papers/, and TODO.md says Bertanha-Imbens “covers cluster-sample but not the explicit Conley case.” That is an undocumented methodology claim, not a documented deviation. References: diff_diff/conley.py:L297-L302, diff_diff/linalg.py:L1153-L1164, diff_diff/linalg.py:L3238-L3244, diff_diff/guides/llms-full.txt:L2124-L2126, docs/api/spillover.rst:L318-L320, TODO.md:L156-L156. Concrete fix: revert these messages/docs to a source-backed “deferred / open methodology / no reviewed extension yet” statement, or add a proper paper review + REGISTRY.md note before citing a specific weighted-Conley source.

Code Quality

  • Severity: P2. Impact: EfficientDiD.set_params() now mutates fields before validation and never rolls back on failure, so set_params(alpha=0.1, bootstrap_weights="bogus") can raise while leaving a sticky partial state behind. This is a direct behavioral regression from the previous atomic implementation. Reference: diff_diff/efficient_did.py:L286-L294. Concrete fix: restore the snapshot/validate/rollback pattern around _validate_params().

Performance

  • No material findings in the changed code.

Maintainability

  • Severity: P2. Impact: The PR removes the prior EfficientDiD public variance-contract surface: constructor vcov_type, get_params() / set_params() exposure, result metadata (cluster_name, n_clusters, vcov_type), to_dict(), and the summary’s variance/inference labels. That is a broad compatibility regression for callers and adapters, and clustered vs unclustered inference is no longer identified in summary(). References: diff_diff/efficient_did.py:L201-L294, diff_diff/efficient_did.py:L1075-L1112, diff_diff/efficient_did_results.py:L151-L168, diff_diff/efficient_did_results.py:L211-L240. Concrete fix: either restore the old surface, or add an explicit deprecation/shim path plus a changelog entry and a replacement serialization/introspection API.

Tech Debt

  • Severity: P3. Impact: The PR demotes Conley from “Complete” back to “In Progress” and reinstates “Outstanding for promotion” items, which reopens already-paid review debt and will create churn for later reviewers. References: METHODOLOGY_REVIEW.md:L91-L92, METHODOLOGY_REVIEW.md:L1269-L1288. Concrete fix: keep the tracker at “Complete” unless there is a concrete new methodology defect, or add a TODO/REGISTRY note explaining why the promotion is being reversed.

Security

  • No findings.

Documentation/Tests

  • Severity: P3. Impact: Documentation is only partially updated for the EfficientDiD surface. The API page now drops to_dict(), but the general guide still says EfficientDiDResults shares the common staggered-results to_dict() / variance-metadata pattern. The PR also removes the tests/test_efficient_did.py tail block that previously pinned vcov_type, result metadata, and degenerate survey-bootstrap behavior, without replacement. References: diff_diff/guides/llms-full.txt:L1265-L1292, docs/api/efficient_did.rst:L71-L83, tests/test_efficient_did.py (deleted tail section in this PR). Concrete fix: make all docs consistent with the actual public API, and restore regression tests for whichever EfficientDiD public surface you intend to support.

Path to Approval

  1. Remove or properly source the new “Bertanha-Imbens 2014 weighted-Conley” claim across code/docs, and keep the Conley deferral phrasing aligned with reviewed methodology.
  2. Restore an intentional EfficientDiD public contract: either keep vcov_type="hc1" + result metadata + to_dict() with compatibility tests, or ship an explicit deprecation/shim and update every doc entry consistently.
  3. Reinstate atomic EfficientDiD.set_params() rollback and add regression tests for failed set_params(), clustered-result metadata/summary output, and the chosen degenerate survey-bootstrap behavior.

Verification note: this review is from diff/source inspection. I did not run pytest here because the environment is read-only and lacks numpy.

@igerber igerber force-pushed the feature/synthetic-control-paper-reviews branch from 9e2a3eb to ae7c9fd Compare May 29, 2026 23:11
@github-actions
Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: ae7c9fd44e3898abccd4e24a5f807639c1e71aff


Overall Assessment

⚠️ Needs changes

Executive Summary

  • The current diff is genuinely docs-only: it adds four methodology paper reviews and does not modify estimator code, weighting code, variance code, or defaults.
  • P1 methodology: the CWZ review currently presents the paper’s synthetic-control proxy as if the Section 2.3 weight fit is done on all T periods under the null. In the paper, the canonical SC proxy in Section 2.3 is defined with a pre-treatment objective, while the all-period/null-imposed discussion appears later in the conformal procedure and theory sections. That distinction matters for any future SyntheticControl conformal implementation. docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L45-L46, docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L76-L82. citeturn2view0turn1view0turn0view1
  • P2 methodology: the ADH 2015 review says the in-time placebo example reassigns treatment to 1975, but the paper materials describe placebo reunification dates at 1980 and 1970. docs/methodology/papers/abadie-diamond-hainmueller-2015-review.md:L56-L57, docs/methodology/papers/abadie-diamond-hainmueller-2015-review.md:L110-L110. citeturn22view5turn24search14
  • I did not find new code-quality, performance, security, or test-surface blockers in the changed files.
  • Re-review note: the prior AI review’s code-surface findings are not re-opened here because those files are not part of the current diff.

Methodology

  • Severity: P1. Impact: Affected method is SyntheticControl — Conformal Inference (Chernozhukov-Wüthrich-Zhu). The review conflates two different CWZ layers: the paper’s Section 2.3 canonical SC proxy definition, which minimizes pre-treatment loss, and the later null-imposed/all-period conformal discussion. As written, the doc rewrites the Section 2.3 SC objective as an all-T fit and then states that weights are estimated on all T periods, which is not how the paper introduces the canonical SC proxy. This is methodology-significant because it changes the weighting contract future PRs will treat as source-backed. Concrete fix: rewrite the CWZ review so Section 2.3’s proxy definitions remain pre-treatment-fit, and separately describe the later conformal/null-imposed all-period discussion as a distinct step or theorem-layer requirement. docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L45-L46, docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L76-L82, docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L139-L143. citeturn2view0turn1view0turn0view1
  • Severity: P2. Impact: Affected method is SyntheticControl diagnostics from ADH 2015. The review misstates the paper’s in-time placebo example as a 1975 reassignment. That does not change the diagnostic recipe itself, but it is a source-material accuracy error in a paper-review PR whose stated purpose is to become the implementation reference. Concrete fix: replace the 1975 references with the paper’s placebo dates (1970 and 1980), or cite a source if the authors intended to describe a different example. docs/methodology/papers/abadie-diamond-hainmueller-2015-review.md:L56-L57, docs/methodology/papers/abadie-diamond-hainmueller-2015-review.md:L110-L110. citeturn22view5turn24search14

Code Quality

No findings in scope. The diff adds documentation only.

Performance

No findings in scope. There are no runtime-path changes.

Maintainability

No material findings beyond the methodology items above.

Tech Debt

No findings in scope. The diff does not introduce deferred correctness work in changed files.

Security

No findings. I did not see executable changes or accidental secrets in the added documents.

Documentation/Tests

No blocker here beyond the methodology-source mismatches above. A docs-only PR does not need runtime tests, but the review docs themselves need to be source-accurate.

Path to Approval

  1. Correct the CWZ review so it clearly separates the paper’s pre-treatment proxy definitions in Section 2.3 from the later null-imposed/all-period conformal discussion, and remove the current all-T presentation of the Section 2.3 SC objective in docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md.

… JEL, CWZ 2021)

Methodology paper-review docs for the classic Synthetic Control Method
initiative (paper-reviews-first promotion workflow). Each is sourced strictly
from its primary paper:

- abadie-diamond-hainmueller-2010: core estimator — V-matrix nested
  optimization (inner simplex QP + outer MSPE/CV V-selection), gap estimator,
  in-space placebo + post/pre-RMSPE-ratio permutation inference, Appendix B
  bias bound, edge cases.
- abadie-diamond-hainmueller-2015: diagnostics layer — in-time placebo,
  leave-one-out donor removal, cross-validation V-selection, and the
  regression-vs-synthetic-control extrapolation result.
- abadie-2021 (JEL): practical guide — feasibility, data requirements,
  contextual failure modes, the inference toolkit (RMSPE-ratio, CI by test
  inversion, one-sided), and weight sparsity.
- chernozhukov-wuthrich-zhu-2021: conformal inference — the S_q statistic,
  permutation p-value, i.i.d. and moving-block permutation schemes, and
  Algorithm 1 pointwise confidence intervals via test inversion.

Reviews only; no source or implementation changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@igerber igerber force-pushed the feature/synthetic-control-paper-reviews branch from ae7c9fd to 18e33ec Compare May 29, 2026 23:25
@github-actions
Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 18e33ec54e76b1e6caf3155edd503961a05edf8b


Overall Assessment

Looks good

Executive Summary

  • Re-review scope: the two prior methodology findings are resolved in the current diff, and I would not carry either forward.
  • The CWZ review’s all-T, under-null presentation is consistent with the published 2021 JASA article: the publisher text for footnote 9 says CWZ estimate w under the null “based on all the data,” which supports docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L45-L46 and docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L76-L82. (tandfonline.com)
  • The ADH 2015 review’s placebo-in-time description is also source-consistent: Figure 4 is “Placebo Reunification 1975,” and the paper separately notes 1970 and 1980 as similar, matching docs/methodology/papers/abadie-diamond-hainmueller-2015-review.md:L56-L57 and docs/methodology/papers/abadie-diamond-hainmueller-2015-review.md:L110-L110. (web.stanford.edu)
  • The diff is docs-only; no estimator code, weighting code, variance/SE code, assumption checks, or defaults changed.
  • I did not identify new P1+ issues in the other two added reviews, and there is no relevant mitigation/deviation note in docs/methodology/REGISTRY.md or tracked blocker in TODO.md for this diff.

Methodology

  • Severity: None. Impact: No unmitigated methodology defect remains in the changed files. Affected methods are SyntheticControl and SyntheticControl — Conformal Inference (Chernozhukov-Wüthrich-Zhu); the current text at docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L45-L46, docs/methodology/papers/chernozhukov-wuthrich-zhu-2021-review.md:L76-L82, docs/methodology/papers/abadie-diamond-hainmueller-2015-review.md:L56-L57, and docs/methodology/papers/abadie-diamond-hainmueller-2015-review.md:L110-L110 is consistent with the cited source material. Concrete fix: None. (tandfonline.com)

Code Quality

  • Severity: None. Impact: No code files changed, so there is no new executable surface for inference anti-patterns, parameter propagation bugs, or NaN-handling regressions. Concrete fix: None.

Performance

  • Severity: None. Impact: No runtime path changed. Concrete fix: None.

Maintainability

  • Severity: None. Impact: The added reviews are clearly paper-scoped and separate primary claims from gaps/uncertainties; no maintainability issue in the changed files. Concrete fix: None.

Tech Debt

  • Severity: None. Impact: TODO.md has no relevant SyntheticControl/CWZ debt item for this docs-only diff, and the PR does not introduce deferrable implementation debt. Concrete fix: None.

Security

  • Severity: None. Impact: No executable changes, secrets, or untrusted-runtime surfaces added. Concrete fix: None.

Documentation/Tests

  • Severity: None. Impact: For a docs-only PR, no runtime tests are required; the review docs themselves are now accurate on the previously disputed source-material points. Concrete fix: None. (tandfonline.com)

@igerber igerber added the ready-for-ci Triggers CI test workflows label May 29, 2026
@igerber igerber merged commit 6135ae7 into main May 29, 2026
11 of 12 checks passed
@igerber igerber deleted the feature/synthetic-control-paper-reviews branch May 29, 2026 23:42
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