perf(had): hoist Stute bootstrap loop invariants — bit-identical, ~2x

[igerber]

Summary

• stute_test / stute_joint_pretest unweighted Appendix-D bootstrap loops re-derived loop invariants every replicate: a fresh rng.choice per Mammen draw, the d-moments inside _fit_ols_intercept_slope, and the np.unique tie-block indices inside _cvm_statistic.
• Now: Mammen draws are batched through memory-bounded rng.choice calls (_iter_mammen_rows; numpy fills C-order, so the variate stream — and therefore every draw — is identical to per-iteration draws, and peak memory stays bounded instead of a full (B, G) matrix at large G), and the helpers accept precomputed d_moments / tie_blocks (deterministic functions of d computed with identical expressions — tie-safety logic stays single-source).
• Bit-identical: bootstrap_S, cvm_stat, p_value exactly equal the literal per-iteration form. Locked by TestStuteBootstrapHoistedInvariants: a frozen byte-copy of the pre-hoist loop (end-to-end p-value equality on a seed×DGP grid with tie blocks), per-helper equality tests, RNG stream-identity (sequential vs batched vs chunked), and a joint-loop invariance test that monkeypatches back per-iteration draws.
• Measured: 2.5x / 2.1x / 1.9x at G=1e3 / 1e4 / 1e5 (B=999), p-values identical. The per-replicate refit remains the paper-literal O(G) form.
• Resolves the Phase-3 TODO row's ~2x target without its sketched M = I − X(X'X)⁻¹X' materialization — that form is O(G²) memory and would not scale; REGISTRY's Phase-3 Stute notes updated accordingly (the shipped change is loop-invariant hoisting, exactly identical rather than "functionally identical"). Large-G advisory now cites measured post-hoist timings.

Methodology references (required if estimator / math changes)

• Method name(s): de Chaisemartin, D'Haultfœuille et al. HAD Phase-3 Stute (1997) CvM linearity pretest, Appendix-D Mammen wild bootstrap (per-replicate OLS refit form preserved)
• Paper / source link(s): REGISTRY.md § HAD "Note (Phase 3 Stute bootstrap)" (updated in this PR)
• Any intentional deviations from the source (and why): None — the Appendix-D algorithm is unchanged; only loop-invariant computations moved out of the replicate loop, with exact-equality locks.

Validation

• Tests added/updated: tests/test_had_pretests.py::TestStuteBootstrapHoistedInvariants (5 tests: stream identity incl. chunked batches, d-moments equality, tie-blocks equality, frozen literal-loop p-value parity, joint-loop per-iteration-draw invariance).
• Evidence: full tests/test_had_pretests.py passes (250); the pre-existing w=ones(G) weighted-reduction bit-exact lock and survey-path tests are unaffected (survey path untouched).

Security / privacy

• Confirm no secrets/PII in this PR: Yes

---

Generated with Claude Code

[github-actions]

PR Review Report

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected methods: HAD Phase-3 Stute CvM pretests, stute_test and stute_joint_pretest.
• The PR preserves the Appendix-D bootstrap contract: Mammen multipliers still form dy_b = fitted + eps * eta, and each replicate still refits OLS.
• The optimization is documented in docs/methodology/REGISTRY.md:L3423-L3428 as loop-invariant hoisting, not a methodology deviation.
• No new inline inference, NaN-guard, variance/SE, or control-group logic issues found.
• I could not run tests locally because pytest is not installed in this environment.

Methodology

• Severity: P3 informational
  Impact: The changed implementation affects Stute bootstrap execution for stute_test and stute_joint_pretest, but the registry explicitly documents the hoisted Mammen draw batching, precomputed d_moments, and precomputed tie blocks. The code still matches the cited Appendix-D per-replicate refit structure. See diff_diff/had_pretests.py:L1991-L2010, diff_diff/had_pretests.py:L3342-L3361, and docs/methodology/REGISTRY.md:L3423-L3428.
  Concrete fix: None required.

Code Quality

• Severity: None
  Impact: The new private helper surfaces are narrowly scoped and use deterministic precomputed values from the same expressions used by the original code. See diff_diff/had_pretests.py:L971-L1002 and diff_diff/had_pretests.py:L1107-L1160.
  Concrete fix: None.

Performance

• Severity: None
  Impact: The change removes repeated invariant work in the unweighted bootstrap while keeping memory bounded via _iter_mammen_rows. See diff_diff/had_pretests.py:L1082-L1104.
  Concrete fix: None.

Maintainability

• Severity: None
  Impact: The registry, docstrings, and tests now describe the performance contract and bit-identity expectations consistently.
  Concrete fix: None.

Tech Debt

• Severity: None
  Impact: The stale Stute performance TODO row was removed after the documented implementation landed; no untracked deferral identified.
  Concrete fix: None.

Security

• Severity: None
  Impact: No secret material, external I/O, or security-sensitive logic appears in the changed code.
  Concrete fix: None.

Documentation/Tests

• Severity: P3 informational
  Impact: Added parity tests cover RNG stream identity, precomputed helper equality, single-test parity, and joint-test parity. See tests/test_had_pretests.py:L5884-L6013.
  Concrete fix: None required. Local verification was blocked by missing pytest on PATH.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: a234af1ca6e093e414d92f20263891b42a594ee7

---

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected methods: HAD Phase-3 Stute CvM bootstrap paths in stute_test and stute_joint_pretest.
• The implementation preserves the Appendix-D contract: draw Mammen multipliers, form dy_b = fitted + eps * eta, refit OLS, recompute CvM per replicate.
• The loop-invariant hoist is explicitly documented in the Methodology Registry as a Phase-3 Stute bootstrap note, so this is not an undocumented methodology deviation.
• No new inline inference, NaN/Inf propagation, variance/SE, control-group, or public parameter propagation issues found.
• Local test execution was blocked: this environment lacks pytest and numpy.

Methodology

• Severity: P3 informational
  Impact: The PR changes execution of the unweighted Stute bootstrap for stute_test and stute_joint_pretest, but the Registry documents the change as loop-invariant hoisting, not a methodology deviation. The code still performs the paper-literal per-replicate refit and CvM recomputation. See docs/methodology/REGISTRY.md:L3425-L3430, diff_diff/had_pretests.py:L1991-L2010, and diff_diff/had_pretests.py:L3342-L3361.
  Concrete fix: None required.

Code Quality

• Severity: None
  Impact: New private helper arguments are narrowly scoped and documented: precomputed d_moments in _fit_ols_intercept_slope, precomputed tie_blocks in _cvm_statistic, and batched rows in _iter_mammen_rows. See diff_diff/had_pretests.py:L971-L1002 and diff_diff/had_pretests.py:L1082-L1163.
  Concrete fix: None.

Performance

• Severity: None
  Impact: The change removes repeated invariant work and keeps Mammen batching memory-bounded through max_batch_bytes, avoiding the deferred O(G²) matrix materialization. See diff_diff/had_pretests.py:L1082-L1104 and CHANGELOG.md:L435-L447.
  Concrete fix: None.

Maintainability

• Severity: None
  Impact: The implementation comments, helper docstrings, Registry note, changelog entry, and tests all describe the same contract: bit-identical batching plus precomputed deterministic invariants.
  Concrete fix: None.

Tech Debt

• Severity: None
  Impact: The prior Phase-3 Stute performance TODO is resolved by the implementation; no new untracked deferred work appears in the changed surface.
  Concrete fix: None.

Security

• Severity: None
  Impact: No external I/O, secret material, credential-like strings, or security-sensitive behavior found in the changed files.
  Concrete fix: None.

Documentation/Tests

• Severity: P3 informational
  Impact: Added tests cover RNG stream identity, d-moment parity, tie-block parity, single-test literal-loop parity, and joint-loop parity. See tests/test_had_pretests.py:L5884-L6013. I could not run them locally because pytest is not installed, and direct import checks also fail because numpy is missing in this environment.
  Concrete fix: None for the PR; rely on CI or run the focused test class in an environment with project dependencies installed.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: a6c74b47e7fb606baef44cd98f672c138f7e5ee6

---

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected methods: HAD Phase-3 Stute CvM bootstrap paths in stute_test and stute_joint_pretest.
• The change is documented in docs/methodology/REGISTRY.md:L3431-L3436 as loop-invariant hoisting, not a methodology deviation.
• The unweighted bootstrap still draws Mammen multipliers, forms dy_b = fitted + eps * eta, refits OLS, and recomputes CvM per replicate.
• No new inline inference, NaN/Inf propagation, variance/SE, public parameter propagation, or control-group issues found.
• I could not run tests locally because this environment lacks numpy, pytest, pandas, and scipy.

Methodology

• Severity: P3 informational
  Impact: The PR changes execution of the unweighted Stute bootstrap loops, but the Registry explicitly documents the change and the code preserves the Appendix-D bootstrap contract. stute_test precomputes deterministic d moments and tie blocks, batches Mammen rows, then performs the same per-replicate OLS refit and CvM recomputation. stute_joint_pretest similarly preserves shared multipliers across horizons. See diff_diff/had_pretests.py:L1991-L2010, diff_diff/had_pretests.py:L3342-L3361, and docs/methodology/REGISTRY.md:L3431-L3436.
  Concrete fix: None required.

Code Quality

• Severity: None
  Impact: New helper arguments are private, narrow, and documented: d_moments on _fit_ols_intercept_slope, tie_blocks on _cvm_statistic, and _iter_mammen_rows for bounded batching. Existing callers remain compatible. See diff_diff/had_pretests.py:L971-L1002 and diff_diff/had_pretests.py:L1082-L1163.
  Concrete fix: None.

Performance

• Severity: None
  Impact: The implementation removes repeated invariant work without introducing the previously deferred O(G^2) residual-maker matrix. Batching is memory-bounded by max_batch_bytes. See diff_diff/had_pretests.py:L1082-L1104 and CHANGELOG.md:L458-L468.
  Concrete fix: None.

Maintainability

• Severity: P3 informational
  Impact: One older Registry note still describes the unweighted parity comparison as survey batching versus “per-iteration _generate_mammen_weights,” while the updated code now uses memory-bounded batched _generate_mammen_weights with per-iteration stream identity. This is not a methodology defect because the new Phase-3 note documents the actual behavior, but the older sentence is slightly stale. See docs/methodology/REGISTRY.md:L3324 and diff_diff/had_pretests.py:L1741-L1748.
  Concrete fix: Optionally update that Registry sentence to say the unweighted path uses a batched _generate_mammen_weights stream that is identical to per-iteration draws.

Tech Debt

• Severity: P3 informational
  Impact: The prior Stute performance TODO is removed, and the remaining multiplier-bootstrap chunking gap is tracked in TODO.md, so it is accepted deferred work under the project rules. See TODO.md:L54-L54.
  Concrete fix: None required.

Security

• Severity: None
  Impact: No external I/O, credential handling, subprocess execution, or secret-like material found in the changed surface.
  Concrete fix: None.

Documentation/Tests

• Severity: P3 informational
  Impact: Added tests cover batched/chunked RNG stream identity, precomputed OLS moment parity, precomputed tie-block parity, single-test literal-loop parity, and joint-loop parity. See tests/test_had_pretests.py:L5884-L6013. Tests were not run locally because required dependencies are unavailable.
  Concrete fix: None for the PR; rely on CI or run tests/test_had_pretests.py::TestStuteBootstrapHoistedInvariants in a dependency-complete environment.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 772f0a095004c50236772ff40e3da03ebc347a6b

---

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected methods: HAD Phase-3 Stute CvM bootstrap paths in stute_test and stute_joint_pretest.
• The methodology registry documents the loop-invariant hoist as non-deviating from Appendix-D: per-replicate OLS refits remain in place.
• Batched Mammen draws, precomputed d moments, and precomputed tie blocks preserve the same bootstrap law and are regression-tested for bit identity.
• No new inline inference, variance/SE, public parameter propagation, NaN/Inf, or control-group issues found.
• Could not run the focused tests locally: pytest is not installed in this environment.

Methodology

• Severity: P3 informational
  Impact: The PR changes bootstrap execution mechanics, but the Registry explicitly documents this as loop-invariant hoisting, not a methodological deviation. The code still forms dy_b = fitted + eps * eta, refits OLS per replicate, and recomputes CvM per replicate. See docs/methodology/REGISTRY.md:L3431-L3436, diff_diff/had_pretests.py:L1991-L2010, and diff_diff/had_pretests.py:L3342-L3361.
  Concrete fix: None required.

Code Quality

• Severity: None
  Impact: Private helper extensions are narrow and backward-compatible: _fit_ols_intercept_slope(..., d_moments=...), _cvm_statistic(..., tie_blocks=...), and _iter_mammen_rows(...). Existing callers remain valid.
  Concrete fix: None.

Performance

• Severity: None
  Impact: The PR removes repeated invariant work and avoids the previously deferred O(G²) matrix materialization. _iter_mammen_rows bounds batch memory via max_batch_bytes. See diff_diff/had_pretests.py:L1082-L1104.
  Concrete fix: None.

Maintainability

• Severity: P3 informational
  Impact: The prior documentation nit remains: an older Registry paragraph still describes survey-vs-unweighted parity as “batched generate_survey_multiplier_weights_batch vs per-iteration _generate_mammen_weights” and says the unweighted code path is preserved bit-exactly. The new Phase-3 note is correct, but this older sentence is stale after the hoist. See docs/methodology/REGISTRY.md:L3324.
  Concrete fix: Optionally update that sentence to say the unweighted path uses a memory-bounded batched _generate_mammen_weights stream that is identical to per-iteration draws.

Tech Debt

• Severity: P3 informational
  Impact: The former Stute performance TODO was removed, and the remaining multiplier-bootstrap chunking gap is tracked in TODO.md:L53, so it is accepted deferred work under the review rules.
  Concrete fix: None required.

Security

• Severity: None
  Impact: No external I/O, subprocess execution, credential handling, or secret-like material in the changed surface.
  Concrete fix: None.

Documentation/Tests

• Severity: P3 informational
  Impact: Added tests cover batched/chunked RNG stream identity, OLS moment parity, CvM tie-block parity, single-test literal-loop parity, and joint-loop per-iteration parity. See tests/test_had_pretests.py:L5884-L6013. I could not execute them locally because pytest is not installed.
  Concrete fix: None for the PR; rely on CI or run tests/test_had_pretests.py::TestStuteBootstrapHoistedInvariants in a dependency-complete environment.