Skip to content

test(callaway): fold ipw R yardsticks into regenerated golden; record no-cov ipw document-only decision#638

Merged
igerber merged 1 commit into
mainfrom
test/csdid-ipw-golden-fold
Jul 7, 2026
Merged

test(callaway): fold ipw R yardsticks into regenerated golden; record no-cov ipw document-only decision#638
igerber merged 1 commit into
mainfrom
test/csdid-ipw-golden-fold

Conversation

@igerber

@igerber igerber commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • Golden fixture regenerated (benchmarks/data/csdid_golden_values.json; R 4.5.2, did 2.5.1, DRDID 1.3.0 — exactly the provenance stack of the hardcoded yardsticks): all pre-existing data blocks and result values reproduced byte-identically (462/462 floats), and the with_covariates_ipw scenario now carries the aggte simple/dynamic/group blocks the generator already emitted. The committed diff is +18 JSON lines, nothing else.
  • test_golden_ipw_aggregation_se_vs_r_did_251 now reads the yardsticks from the JSON (values identical to the previously hardcoded 2026-07-05 numbers; same 1e-6 tolerance, observed agreement ~1e-10), with a skip guard for pre-regeneration fixtures.
  • No-covariate ipw structural-parity decision recorded (document-only): R did fits an intercept-only logit whose estimation-effect correction is identically zero at the MLE; mirroring it would add a per-cell IRLS solve (and non-convergence failure surface) for zero numerical change. REGISTRY § CallawaySantAnna now carries an explicit **Note:**; a new TestDRNoCovariateSEUniformity::test_ipw_no_cov_per_cell_identical_to_reg locks no-cov ipw == reg bit-identical per cell and on aggregated surfaces.
  • Closes the two CS-scaling Quick TODO rows (ipw yardstick folding + no-cov ipw decision).

Methodology references (required if estimator / math changes)

  • Method name(s): Callaway & Sant'Anna (2021) group-time ATT, IPW estimation method (Abadie 2005 / DRDID std_ipw_did_panel). No estimator code changed — fixture, tests, and REGISTRY documentation only.
  • Paper / source link(s): REGISTRY.md § CallawaySantAnna (ipw notes); R did 2.5.1 / DRDID 1.3.0 as the parity reference.
  • Any intentional deviations from the source (and why): the no-covariate ipw unconditional-propensity treatment is now an explicitly documented deviation (REGISTRY **Note:**, decided document-only) — numerically identical to R, structurally simpler, locked by a bit-identity test.

Validation

  • Tests added/updated: tests/test_csdid_ported.py (golden-reading rewrite), tests/test_methodology_callaway.py (new ipw/reg bit-identity test).
  • Evidence: regenerated fixture cross-checked against the committed one (all shared values byte-equal; new blocks equal the hardcoded yardsticks exactly); tests/test_csdid_ported.py + tests/test_methodology_callaway.py pass locally (145 tests, both backends' dispatch unchanged).

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

… record no-cov ipw document-only decision

csdid_golden_values.json regenerated with R 4.5.2 / did 2.5.1 / DRDID
1.3.0: all pre-existing data and result blocks reproduced
byte-identically (462/462 floats), and the with_covariates_ipw scenario
now carries the aggte simple/dynamic/group blocks — identical to the
previously hardcoded 2026-07-05 yardsticks, which
test_golden_ipw_aggregation_se_vs_r_did_251 now reads from the JSON.

The no-covariate ipw unconditional-propensity treatment is recorded as
a document-only decision in REGISTRY (R did's intercept-only logit is
deliberately not mirrored — its estimation-effect correction is
identically zero at the MLE), locked by a new bit-identity test
(no-cov ipw == reg per cell and aggregated).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Overall Assessment

Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • Affected method: CallawaySantAnna panel IPW parity coverage and the documented no-covariate IPW structural decision; estimator code is unchanged.
  • The no-covariate IPW deviation is explicitly documented with a **Note:** label in the Methodology Registry, so it is mitigated under the review rules.
  • The added golden fixture blocks are wired through the R generator and consumed by the updated IPW aggregation parity test.
  • No new inline inference, partial NaN guard, parameter propagation, control-group, or security issues were introduced.
  • I could not execute the targeted tests locally because pytest is not installed in this runner.

Methodology

  • Severity: P3 informational, no action required
    Location: docs/methodology/REGISTRY.md:L522-L549, diff_diff/staggered.py:L3297-L3331, tests/test_methodology_callaway.py:L425-L455
    Impact: The PR records that no-covariate IPW uses the unconditional difference-in-means IF rather than structurally mirroring R’s intercept-only logit. This is a documented deviation with the required **Note:** label, and the implementation path already reduces no-covariate IPW to difference in means. R did’s panel IPW path delegates to DRDID::std_ipw_did_panel, and DRDID’s standardized IPW path is a Hajek estimator with logit propensity estimation, an M2 propensity-score estimation-effect term, and IF-based SEs; with an intercept-only design, the documented zero-correction rationale is consistent with that source structure. (rdrr.io)
    Concrete fix: None required.

Code Quality

No findings. The test rewrite cleanly replaces hardcoded IPW aggregation yardsticks with JSON-backed expected values and keeps explicit skip guards for older fixtures: tests/test_csdid_ported.py:L1484-L1530.

Performance

No findings. The PR changes fixtures, tests, docs, and the R generator comments/output shape; no runtime estimator hot path is modified.

Maintainability

No findings. The generator now emits the same IPW aggregation blocks consumed by the Python test, reducing duplicated hardcoded values: benchmarks/R/generate_csdid_test_values.R:L111-L142.

Tech Debt

No findings. The removed TODO rows correspond to work completed or converted into a documented Registry decision: TODO.md:L25-L35, docs/methodology/REGISTRY.md:L544-L549.

Security

No findings. The changed files contain simulation data, docs, tests, and generator code; no credible secret material was introduced.

Documentation/Tests

No blocking findings. The fixture contains the new simple, dynamic, and group IPW aggregation blocks at benchmarks/data/csdid_golden_values.json:L94-L118, and the new methodology test locks no-covariate IPW against the regression path per cell and overall at tests/test_methodology_callaway.py:L425-L455.

Validation note: targeted test execution was attempted with pytest and python -m pytest, but pytest is unavailable in this environment.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 7, 2026
@igerber igerber merged commit 896becc into main Jul 7, 2026
31 of 32 checks passed
@igerber igerber deleted the test/csdid-ipw-golden-fold branch July 7, 2026 21:07
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