perf(linalg): low-rank factored CR2-BM adjustment operators (8-108x)

[igerber]

Summary

• Unweighted clustered CR2 Bell-McCaffrey adjustment operators A_g = (I − H_gg)^{−1/2} are evaluated via a low-rank identity: H_gg = X_g M_U X_g' has rank ≤ k, so A_g = I + (U_g Q) diag(γ) (U_g Q)' from the k×k eigenproblem of U_g'U_g (U_g = X_g M_U^{1/2}), with Moore-Penrose zeroing at 1−λ ≤ 1e-10 matching _cr2_adjustment_matrix's convention. Consumers only ever apply A_g to skinny matrices, so the dense (n_g, n_g) A_g — an O(n_g³) eigh per cluster, ~85% of CR2-BM runtime — is never materialized; γ is evaluated via expm1/log1p (stable as λ→0). The unweighted S_W/MUWTWUM build is also skipped (S_W = X'X exactly).
• End-to-end _compute_cr2_bm: 18→2.3 ms (n=5k/G=50/k=10), 119→5.3 ms (n=20k/G=100), 597→33 ms (n=100k/G=500), 4111→38 ms, ~108x (n=100k/G=100/k=40).
• Weighted (clubSandwich WLS-CR2) path unchanged — its G_g carries the S_W bias term and is not a rank-k identity perturbation.
• _cr2_bm_dof_inner now takes the precomputed A_g_Xbi blocks; the MPD build-once spy re-anchored on the shared core (the dense per-cluster build no longer exists to count).
• Rust CR2-BM TODO row re-scoped to Parked: the NumPy path is now BLAS-bound, so the port buys ~nothing (user-approved direction change).

Methodology references (required if estimator / math changes)

• Method name(s): CR2 Bell-McCaffrey cluster-robust variance + Satterthwaite DOF (unweighted path) — evaluation change only, no estimand/formula change.
• Paper / source link(s): Bell & McCaffrey (2002); Pustejovsky & Tipton (2018), J. Bus. Econ. Statist. 36(4) (§3.1 Eq. 11 scalar Satterthwaite DOF); Imbens & Kolesár (2016).
• Any intentional deviations from the source (and why): None — algebraically identical evaluation; REGISTRY §DifferenceInDifferences CR2-BM Evaluation note extended (low-rank A_g factorization, pseudoinverse convention mapping). vcov/DOF match a frozen dense-eigh oracle at rtol 1e-12/1e-10 with identical NaN-guard patterns.

Validation

• Tests added/updated: tests/test_linalg.py::TestCR2BMLowRankAdjustment (frozen dense-eigh oracle across balanced/unbalanced/k=40/leverage-1 absorbed-cluster-FE/singletons + near-zero-leverage γ-stability); tests/test_estimators_vcov_type.py::TestMPDClusterHC2BMSharedPrecompute spy re-anchored on the shared core.
• Backtest / simulation / notebook evidence (if applicable): benchmark table in the CHANGELOG entry (measured on Apple Accelerate; 3-rep minimums).
• 281 linalg+vcov tests, clubSandwich WLS-CR2 goldens, and 682 estimator-suite tests pass unchanged on both backends (rust + pure Python).

Security / privacy

• Confirm no secrets/PII in this PR: Yes

---

Generated with Claude Code

[github-actions]

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected method: unweighted clustered CR2 Bell-McCaffrey variance + Satterthwaite DOF in diff_diff/linalg.py.
• The low-rank A_g path is an evaluation change, not an estimand/formula change, and is documented in docs/methodology/REGISTRY.md:L247.
• Weighted WLS-CR2 remains on the dense clubSandwich path, which is appropriate because its G_g includes the weighted bias term.
• No new inline inference, partial NaN guard, parameter propagation, or control-group logic issues found.
• I could not execute tests here because both pytest and numpy are unavailable in the review environment.

Methodology

• Severity: P3 informational, no action required
  Impact: The changed unweighted path evaluates the same CR2 adjustment operator A_g = (I - H_gg)^(-1/2) through a low-rank factorization and keeps the same Satterthwaite DOF helper. This is consistent with the registry note at docs/methodology/REGISTRY.md:L247 and the in-code derivation at diff_diff/linalg.py:L2029-L2082. The cited source material supports CR2/BRL as the Bell-McCaffrey/Pustejovsky-Tipton small-sample adjustment and Satterthwaite coefficient tests as the relevant inference surface. (arxiv.org)
  Concrete fix: None.

Code Quality

• Severity: None
  Impact: The refactor keeps weighted and unweighted CR2 logic separated cleanly, with _cr2_bm_dof_inner now receiving precomputed A_g @ X_g @ bread_inv blocks as documented at diff_diff/linalg.py:L2218-L2281. No new public parameters or incomplete propagation issues found.
  Concrete fix: No action.

Performance

• Severity: None
  Impact: The new path avoids dense per-cluster (n_g, n_g) eigendecompositions and materializes only skinny applications, matching the stated performance intent at diff_diff/linalg.py:L2045-L2052.
  Concrete fix: No action.

Maintainability

• Severity: None
  Impact: The registry, changelog, and tests all describe the same evaluation change. The MPD spy test was re-anchored to the shared CR2 core at tests/test_estimators_vcov_type.py:L2476-L2508, which matches the new low-rank implementation where _cr2_adjustment_matrix is no longer called on the unweighted path.
  Concrete fix: No action.

Tech Debt

• Severity: P3 informational, tracked
  Impact: The Rust CR2-BM port was moved from actionable backlog to parked/deferred work in TODO.md, with the rationale documented under the parked section. This is deferrable performance work, not a correctness blocker.
  Concrete fix: None.

Security

• Severity: None
  Impact: No secrets or sensitive material found in the changed files.
  Concrete fix: No action.

Documentation/Tests

• Severity: None
  Impact: Added tests cover dense-oracle parity for balanced, unbalanced, high-k, leverage-1 absorbed-FE, singleton-cluster, and near-zero-leverage cases at tests/test_linalg.py:L2719-L2814.
  Concrete fix: No action.

Verification note: git diff --check passed. Focused test execution was not possible because this environment lacks pytest and numpy.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 6b13e7de547e98992490f39f91ca38a70491b607

---

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected method: unweighted clustered CR2 Bell-McCaffrey variance and Satterthwaite DOF.
• The low-rank A_g evaluation is documented in docs/methodology/REGISTRY.md:L247 as an algebraically identical implementation change.
• Weighted WLS-CR2 remains on the dense clubSandwich path at diff_diff/linalg.py:L2084-L2130.
• No new inline inference, partial NaN guard, parameter propagation, or control-group issues found.
• Re-review note: prior AI review had no P0/P1 findings; I found no regression from that assessment.
• Verification was limited: git diff --check passed, but tests could not run because pytest is not installed and numpy import fails.

Methodology

• Severity: P3 informational, no action required
  Impact: The unweighted path now evaluates A_g = (I - H_gg)^(-1/2) through the documented low-rank identity at diff_diff/linalg.py:L2029-L2082, while retaining the same CR2-BM sandwich and Satterthwaite DOF surface. This aligns with the registry note at docs/methodology/REGISTRY.md:L247. The external reference surface also matches the intended method family: clubSandwich documents CR2 as the Bell-McCaffrey bias-reduced linearization adjustment and reports Satterthwaite degrees of freedom for coefficient tests. (jepusto.github.io) The clubSandwich source computes Satterthwaite DF from the P-array trace/sum-of-squares form, consistent with the local weighted path documentation. (raw.githubusercontent.com)
  Concrete fix: None.

Code Quality

• Severity: None
  Impact: The branch split is clear: unweighted low-rank factors at diff_diff/linalg.py:L2029-L2082, weighted dense clubSandwich construction at diff_diff/linalg.py:L2084-L2130, and _cr2_bm_dof_inner now accepts precomputed A_g @ X_g @ bread_inv blocks at diff_diff/linalg.py:L2218-L2291. No missing caller propagation found.
  Concrete fix: No action.

Performance

• Severity: None
  Impact: The new path avoids dense per-cluster (n_g, n_g) eigendecompositions and materializes only skinny applications, matching the documented performance intent in diff_diff/linalg.py:L2042-L2052.
  Concrete fix: No action.

Maintainability

• Severity: None
  Impact: The MPD precompute spy was correctly re-anchored to _compute_cr2_bm_vcov_and_dof because _cr2_adjustment_matrix is no longer expected on the unweighted path, see tests/test_estimators_vcov_type.py:L2476-L2508.
  Concrete fix: No action.

Tech Debt

• Severity: P3 informational, tracked
  Impact: The Rust CR2-BM port was moved to parked/deferred work with rationale in TODO.md:L112-L114. This is tracked performance work, not a correctness blocker.
  Concrete fix: None.

Security

• Severity: None
  Impact: No secrets, unsafe IO, or security-sensitive changes found in the diff.
  Concrete fix: No action.

Documentation/Tests

• Severity: None
  Impact: New tests cover dense-oracle parity for balanced, unbalanced, high-k, leverage-1 absorbed-FE, singleton, and near-zero-leverage cases at tests/test_linalg.py:L2719-L2814. Existing contrast-oracle tests cover arbitrary contrast DOF at tests/test_linalg.py:L2875-L2891.
  Concrete fix: No action.

Verification: git diff --check passed. Focused tests were not executable in this environment: pytest was not found, and numpy is not installed.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: b9991843e4cad5e8eea6897eef402881e4e2e4af

---

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings. I found one P2 performance edge case worth addressing or tracking, but it is not a correctness blocker.

Executive Summary

• Affected method: unweighted clustered CR2 Bell-McCaffrey variance and Satterthwaite DOF.
• The low-rank A_g evaluation is documented in docs/methodology/REGISTRY.md:L247 as an algebraically identical implementation change.
• The code preserves the dense-path pseudoinverse convention for 1 - lambda <= 1e-10 at diff_diff/linalg.py:L2063-L2068.
• Weighted WLS-CR2 remains on the dense clubSandwich path at diff_diff/linalg.py:L2084-L2132.
• No new inline inference, partial NaN guard, parameter propagation, or control-group issues found.
• Re-review note: prior AI review had no P0/P1 findings; I found no regression from that assessment.

Methodology

Severity: P3 informational, no action required
Impact: The PR changes evaluation of unweighted A_g = (I - H_gg)^(-1/2), not the estimand or variance formula. This matches the registry note at docs/methodology/REGISTRY.md:L247 and the in-code contract at diff_diff/linalg.py:L2030-L2081. Source material supports the CR2/BRL adjustment matrix and Satterthwaite DOF surface: Pustejovsky-Tipton describe CRVE with adjustment matrices A_i, the unweighted identity-working-model simplification to (I_i - X_i M_X X_i')^(-1/2), and Eq. 11 Satterthwaite degrees of freedom; clubSandwich documents CR2 as Bell-McCaffrey bias-reduced linearization and coef_test(..., test="Satterthwaite") as reporting Satterthwaite df/p-values. (arxiv.org)
Concrete fix: None.

Code Quality

Severity: None
Impact: The helper signature change is consistently propagated: _cr2_bm_dof_inner now accepts precomputed A_g_Xbi blocks at diff_diff/linalg.py:L2218-L2224, and callers pass that contract at diff_diff/linalg.py:L2081 and tests/test_linalg.py:L2740-L2749.
Concrete fix: No action.

Performance

Severity: P2
Impact: The new unweighted path always solves a k x k eigenproblem per cluster at diff_diff/linalg.py:L2060-L2061. This is the intended win when clusters are large relative to k, but it can regress when many clusters are tiny and k >> n_g; the previous dense path solved an n_g x n_g eigenproblem in that case. The changelog/registry also describe the path as O(n_g k^2) while the per-cluster eigh(U_g.T @ U_g) adds an O(k^3) term.
Concrete fix: Branch on cluster shape: use the current k x k Gram path when k <= n_g, and a smaller-side n_g x n_g eigen/SVD apply when n_g < k. Add a high-k, small-cluster perf/parity test.

Maintainability

Severity: None
Impact: The MPD precompute spy was reasonably re-anchored from _cr2_adjustment_matrix to _compute_cr2_bm_vcov_and_dof because the unweighted path no longer materializes dense A_g; see tests/test_estimators_vcov_type.py:L2476-L2508.
Concrete fix: No action.

Tech Debt

Severity: P3 informational, tracked
Impact: The Rust CR2-BM port was moved to parked/deferred work with rationale in TODO.md:L112-L114. This is tracked performance work, not a correctness blocker.
Concrete fix: None.

Security

Severity: None
Impact: No secrets, unsafe IO, or security-sensitive changes found in the changed files.
Concrete fix: No action.

Documentation/Tests

Severity: P3
Impact: New dense-oracle tests cover balanced, unbalanced, high-k, leverage-1 absorbed-FE, singleton, and near-zero-leverage cases at tests/test_linalg.py:L2719-L2824. The remaining gap is the performance-shape noted above: singleton/tiny clusters are tested only with small k.
Concrete fix: Add a parity/performance guard for n_g < k, e.g. singleton or 2-row clusters with k=40+, especially if the implementation adds a smaller-side branch.

Verification: git diff --check passed. Focused tests were not executable in this environment because numpy and pytest are not installed.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 828bd187386dce128b64691fb28a125dc27595db

---

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected method: unweighted clustered CR2 Bell-McCaffrey variance and Satterthwaite DOF.
• The low-rank A_g evaluation is documented in the Methodology Registry as an algebraically identical implementation change: docs/methodology/REGISTRY.md:L247.
• The prior P2 small-cluster performance concern is addressed by the n_g <= k dense fallback: diff_diff/linalg.py:L2067-L2078.
• Weighted WLS-CR2 remains on the dense clubSandwich path: diff_diff/linalg.py:L2101-L2151.
• No new inline inference, partial NaN guard, parameter propagation, or control-group issues found.
• Verification: git diff --check passed; focused pytest could not run because pytest is not installed.

Methodology

Severity: P3 informational, no action required
Impact: The PR changes only evaluation of unweighted A_g = (I - H_gg)^(-1/2). The low-rank identity, Moore-Penrose threshold mapping, and weighted-path exclusion are documented in docs/methodology/REGISTRY.md:L247 and implemented at diff_diff/linalg.py:L2030-L2099. This is an implementation choice matching the same CR2-BM operator, not an undocumented methodology deviation.
Concrete fix: None.

Code Quality

Severity: None
Impact: _cr2_bm_dof_inner’s new contract receives precomputed A_g_Xbi blocks, and all direct callers were updated consistently: diff_diff/linalg.py:L2098, diff_diff/linalg.py:L2235-L2241, tests/test_linalg.py:L2740-L2749.
Concrete fix: No action.

Performance

Severity: None
Impact: The previous re-review’s small-cluster regression concern is resolved. The code now uses the tiny dense construction when n_g <= k, avoiding a larger k x k eigenproblem for singleton or small clusters: diff_diff/linalg.py:L2067-L2078. The new parity test includes tiny-clusters-k-gt-ng: tests/test_linalg.py:L2769-L2786.
Concrete fix: No action.

Maintainability

Severity: None
Impact: The shared-core spy was re-anchored appropriately now that unweighted dense per-cluster A_g calls no longer exist for large clusters: tests/test_estimators_vcov_type.py:L2476-L2508.
Concrete fix: No action.

Tech Debt

Severity: P3 informational, tracked
Impact: The Rust CR2-BM port was moved to parked/deferred work with rationale in TODO.md:L108-L114. This is tracked performance scope, not a correctness blocker.
Concrete fix: None.

Security

Severity: None
Impact: No secrets, unsafe IO, or security-sensitive changes found in the changed files.
Concrete fix: No action.

Documentation/Tests

Severity: P3 informational
Impact: Dense-oracle tests now cover balanced, unbalanced, high-k, leverage-1 absorbed-FE, singleton, tiny-cluster, and near-zero leverage cases: tests/test_linalg.py:L2719-L2832. Changelog and registry document the evaluation change and weighted-path limitation: CHANGELOG.md:L622-L643, docs/methodology/REGISTRY.md:L247.
Concrete fix: No action required. Focused tests should be run in CI/local env because this container lacks pytest.