test: anchor LPDiD non-absorbing SEs against the authors' Stata lpdid package - #785
Conversation
Overall assessment: ✅ Looks goodExecutive summary
Methodology
Code QualityNo findings. PerformanceNo findings. The 51 tests share five module-scoped estimator fits. Maintainability
Tech Debt
SecurityNo findings or apparent secrets. Documentation/TestsApart from the P2 documentation conflict above, coverage and parity scoping are strong. Fixture hashes and schemas validate successfully; full tests could not run because the review environment lacks project dependencies and Stata. |
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment: ✅ Looks goodExecutive summary
Methodology
The paper specifies the Eq. 12/13 clean-sample restrictions but no SE formula. Unit-clustered inference is therefore correctly presented as an implementation choice validated against Stata. Code QualityNo findings. Empty/missing values fail closed, and parity comparisons cover points, SEs, and sample sizes. PerformanceNo findings. The 51 parametrized tests reuse five module-scoped fits. MaintainabilityNo findings. The previous provenance issue is resolved: both Locations: Tech Debt
SecurityNo findings or apparent secrets. Documentation/TestsNo findings. The prior pooled-inference documentation conflict is resolved by distinguishing Static Python, JSON, YAML, fixture-hash, and sample-rule checks passed. Runtime tests could not run because pytest, NumPy, pandas, and Stata are unavailable in the review environment. |
igerber
added
ready-for-ci
… package New committed-golden Stata arm running SSC lpdid v1.0.3 end-to-end (the package builds its own clean samples, unlike the hand-reconstructed teffects RA arm). First external anchors for the non-absorbing reweighted SE (previously pinned-only via RW_SE_PIN), the non-absorbing pooled windows (points and SEs), and the Eq. 12 reweighted point. Mapping: nonabsorbing(, firsttreat notyet) == first_entry (Eq. 12); nonabsorbing(L) == effect_stabilization (Eq. 13). Parity scoping: Eq. 12 agrees on every surface of the committed panel (all ES horizons incl. placebos + both pooled windows; att ~1e-14 vw / ~1.3e-8 rw, SE ~6e-17 vw / ~2.3e-9 rw). Eq. 13 agrees at post horizons + pooled post on a convention-neutral 47-unit subsample (att ~1.1e-14 / ~1.9e-8, SE ~1.8e-16 / ~1.6e-9). Three package convention differences measured, adjudicated against the paper's Eq. 13 text, and locked by divergence-documentation gates (att floors + sample-size mismatch): the package requires L+1 untreated periods before re-entry (stricter than Eq. 13's levels condition, which the library implements), admits always-treated units as early controls via missing-lag boundary semantics (paper-silent), and builds placebo samples by recursive lagged intersection (paper-silent). - benchmarks/stata/generate_lpdid_nonabsorbing_golden.do: arms A (Eq. 12 full panel, vw+rw), B (Eq. 13 subsample, vw+rw, preserve/restore- wrapped), C (Eq. 13 full panel, divergence documentation); fail-closed 8-dep guard; _jnum fails closed on missing scalars; version capture widened for headerless packages and ALWAYS embeds the ado checksum+len (fails closed if checksum unavailable) - benchmarks/data/lpdid_nonabsorbing_stata_golden.json: committed golden (source_sha256-pinned to the committed panel; ssc_versions records every guard dependency with embedded checksums) - tests/test_lpdid_nonabsorbing_stata_parity.py: 51 tests, gates 1-8; split fixture guards (the R golden gates only gate 1) - benchmarks/stata/requirements.do: adds lpdid + boottest/egenmore/ listreg (egenmore probed via _gfilter AND _gclsst so an incomplete install is repaired on rerun) - REGISTRY LPDiD: SE block + Deviation 4 rewritten with the measured anchors and the two-tier paper adjudication; pooled-inference description corrected (package fits pooled windows as separate unit-clustered regressions, not suest stacking); boundary bullet extended; checklist split; Note 1 scoped - DEFERRED.md non-absorbing row: Resolved (SE half, scoped); residual narrowed to RA + Eq. 13 placebo/pooled-pre inference - Docs: benchmarks/stata/README.md (new arm section + stale five-arm/ native-claim fixes), benchmarks/README.md, docs/benchmarks.rst, docs/api/lpdid.rst, llms-full.txt scoping, dube-2025-review.md (this-repo annotation), doc-deps.yaml (benchmarks.rst + paper review mapped under lpdid.py), CHANGELOG - tests/test_methodology_lpdid.py: RW_SE_PIN comment + class docstring scoped to the new anchor; tier-2 wording sweep (comments only; R goldens/panels NOT regenerated) No library source change; no pin values change.
…ope the paper-review suest bullets to joint tests with the measured pooled construction
a5527a4 to
9dfd151
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment: ✅ Looks goodExecutive summary
Methodology
The paper specifies no SE formula. Unit-clustered inference is appropriately documented as an implementation choice validated against Stata. Code QualityNo findings. Missing/non-finite golden values fail closed, and no inline inference anti-pattern was introduced. PerformanceNo findings. The 51 parametrized checks reuse five module-scoped fits. MaintainabilityNo findings. The prior provenance gap is resolved: Location: Tech Debt
SecurityNo findings or apparent secrets. Documentation/TestsNo findings. The prior Static AST, JSON, source-hash, provenance, drop-rule, and doc-dependency checks passed. Runtime pytest was unavailable because pytest and NumPy are not installed. |
1 participant
Summary
benchmarks/stata/generate_lpdid_nonabsorbing_golden.do→benchmarks/data/lpdid_nonabsorbing_stata_golden.json→tests/test_lpdid_nonabsorbing_stata_parity.py, 51 tests) anchoring the LPDiD non-absorbing modes against the authors' SSClpdidpackage (v1.0.3) end-to-end — the package builds its own clean samples, unlike the hand-reconstructedteffectsRA arm.RW_SE_PINwith a documented ~5e-5 fixest weighted-cluster convention gap), the non-absorbing pooled windows (points AND SEs), and the Eq. 12 reweighted point.nonabsorbing(, firsttreat notyet)==first_entry(Eq. 12);nonabsorbing(L)==effect_stabilization(Eq. 13).benchmarks/stata/requirements.dogainslpdid+ its deps (boottest/egenmore/listreg); the generator fails closed on all 8 run-time dependencies and records each inmeta.ssc_versionswith an embedded ado checksum+length drift signal.docs/benchmarks.rst,docs/api/lpdid.rst,llms-full.txtscoping, paper-review annotation, doc-deps mappings, CHANGELOG.Measured deviations (library recompute vs committed Stata golden, same machine)
All inside the module gates (att
abs=1e-6, SEabs=1e-7). Sample sizes match exactly on every gated surface (per-horizon and pooled obs). Stata Eq. 12 vw also reproduces the committed Rfixest::feolsgolden to ~1e-13 (gate 1).Convention differences (measured, adjudicated, divergence-gated)
Three reference-package convention differences are documented in REGISTRY
## LPDiDDeviation 4, adjudicated against the paper's stated Eq. 13 restriction (treated:D=0levels on[t-L, t-1]; control:ΔD=0on[-h, L]lags):L+1untreated periods before re-entry; stricter than the paper, whose levels condition the library implements (3 rows on the committed panel).tcontrols; paper-silent surface, both sides documented conventions (30 rows/horizon).The Eq. 13 full-panel and placebo/pooled-pre divergences these produce are recorded in the golden and locked by divergence-documentation gates (att floors + sample-size-mismatch asserts), so parity claims stay scoped; those surfaces remain outside the Stata anchor (tracked in DEFERRED.md along with the non-absorbing RA path).
Methodology references (required if estimator / math changes)
first_entryEq. 12 /effect_stabilizationEq. 13) — validation only, no estimator changes459273v1.0.3## LPDiDDeviation 4 (see above)Validation
tests/test_lpdid_nonabsorbing_stata_parity.py(new, 51 tests, gates 1–8);tests/test_methodology_lpdid.py(comment/docstring scoping only, no pin values changed); full targeted runs pass (test_lpdid_nonabsorbing_stata_parity51,test_lpdid_ra_stata_parity+test_doc_deps_integrity269 combined,test_methodology_lpdid+test_lpdid132)grep -E '^r\([0-9]+\);')Security / privacy