Skip to content

Commit 5f7f6ee

Browse files
igerberclaude
andcommitted
trop: CI codex R5 P3 fixes — paper-version note + Eq. 10 "balancing representation" wording
Address 2 P3 findings from CI codex R5 on PR #491 (verdict was ✅ Looks good; no P0/P1). P3 - Paper version pinning: paper review on file is v2-pinned (arXiv:2508.21536v2) but the current arXiv version is v3. Added an explicit "Version-pinning note (2026-05-25)" to the paper review file acknowledging that the methodology promotion ships against v2 and that a formal v2-vs-v3 delta-check has NOT been performed. Articulated the action item: refresh the review against the most recent arXiv version when the paper-author reference implementation lands ("forthcoming") and re-validate the verified-component checklist. This is honest deferral rather than an unverified "no methodology changes" claim. P3 - Eq. 10 wording drift: my new prose called Eq. 10 a "paper-side asymptotic identity" but the paper review on file describes it as a "balancing representation / decomposition" (per paper Section 5.2). Replaced "asymptotic identity" with "balancing representation" / "balancing representation / decomposition" across tests/test_methodology_trop.py (module + class docstrings), METHODOLOGY_REVIEW.md (Verified Components Eq. 2 bullet), and docs/methodology/REGISTRY.md (Eq. 10 note). CHANGELOG already used "balancing-decomposition pointer" — no change there. No source-code changes to diff_diff/trop*.py. 36 methodology tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4481836 commit 5f7f6ee

4 files changed

Lines changed: 6 additions & 4 deletions

File tree

METHODOLOGY_REVIEW.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ These three are feature deferrals (paper-supported extensions that the library h
833833

834834
**Verified Components:**
835835

836-
- [x] Eq. 2 weighted nuclear-norm-penalised L estimation: proximal-gradient inner solver (soft-threshold SVD) converges to ``prox_{λ/2}(R)`` under uniform weights; **plain** (non-accelerated) prox-gradient objective ``f(L) + λ‖L‖_*`` is non-increasing across iterations (this verifies the prox + gradient ingredient, NOT the shipped accelerated FISTA outer loop — Nesterov momentum gives the faster ``O(1/k^2)`` rate but does not guarantee per-step monotonicity); weighted-prox under non-uniform weights monotonically decreases the weighted objective and reduces total singular-value mass. (Eq. 10 balancing decomposition is the paper's asymptotic identity built from these ingredients; direct numerical reconstruction is out of scope — see "Outstanding Concerns".)
836+
- [x] Eq. 2 weighted nuclear-norm-penalised L estimation: proximal-gradient inner solver (soft-threshold SVD) converges to ``prox_{λ/2}(R)`` under uniform weights; **plain** (non-accelerated) prox-gradient objective ``f(L) + λ‖L‖_*`` is non-increasing across iterations (this verifies the prox + gradient ingredient, NOT the shipped accelerated FISTA outer loop — Nesterov momentum gives the faster ``O(1/k^2)`` rate but does not guarantee per-step monotonicity); weighted-prox under non-uniform weights monotonically decreases the weighted objective and reduces total singular-value mass. (Eq. 10 balancing representation / decomposition is the paper's identity built from these ingredients per Section 5.2; direct numerical reconstruction is out of scope — see "Outstanding Concerns".)
837837
- [x] Eq. 3 per-(i, t) weights: unit distance excludes the target period (``1{u ≠ t}`` mask in the kernel) and uses only periods where both units are untreated (``(1 - W_iu)(1 - W_ju)`` mask).
838838
- [x] Eqs. 4-5 + Algorithm 1 LOOCV: ``Q(λ)`` sums squared pseudo-treatment effects over ALL control observations where ``D_js = 0`` (including pre-treatment cells of eventually-treated units, paper Eq. 2 control set); two-stage coordinate-descent cycling (footnote 2) returns a tuple of values from the input grids.
839839
- [x] Corollary 1 (paper p. 23) — **single-draw sanity checks consistent with the three unbiasedness conditions, not a repeated-MC mean-bias study**: each of the three balance conditions (a) unit balance, (b) time balance, (c) ``B = 0`` is exercised on a targeted DGP that makes one condition trivially hold while keeping the others sub-optimal. The assertion in each case is a single-realisation ``|att - τ| < 3 * se`` band using the estimator's own bootstrap SE — this is a smoke check, NOT a repeated-draw Monte Carlo bias study of the paper's conditional-unbiasedness statement under fixed weights. A stronger MC bias study at fixed λ values is deferred (would multiply test runtime by ~30x for marginal additional evidence given the existing 3-σ band already catches order-of-magnitude bias regressions).

docs/methodology/REGISTRY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2389,7 +2389,7 @@ Q(λ) = Σ_{j,s: D_js=0} [τ̂_js^loocv(λ)]²
23892389
- [x] Unbalanced panels supported — missing control / pre-treatment cells don't trigger false absorbing-state violations. Locked by `tests/test_methodology_trop.py::TestTROPDeviations::test_unbalanced_panels_supported` (10% random drops on control + pre-treatment subset). Missing-treated-cell and thinner-donor-support edge cases are covered defensively in `tests/test_trop.py::TestDMatrixValidation`.
23902390
- [x] Per-observation treatment-effect estimation (Eq. 13 / Algorithm 2) — `treatment_effects` dict contains one finite `τ_hat_it` per treated cell, and the aggregate ATT equals the unweighted mean of per-cell effects (Eq. 1). **The methodology test exercises block adoption with a constant treatment effect**; **absorbing-state staggered adoption** and **heterogeneous per-cell effects** (paper Remark 6.1) are SUPPORTED by the code path (the implementation does not gate on cohort or effect-magnitude pattern), but are not directly verified in the methodology test surface in this PR. **Section 6.1 non-absorbing / on-off / switching assignment patterns are explicitly OUT OF SCOPE** — the absorbing-state validator at `trop_local.py` rejects non-monotonic D matrices with a `ValueError`, and `TestTROPDeviations::test_event_style_d_rejected_with_value_error` enforces the rejection contract (event-style D being one specific non-absorbing pattern; the same validator catches all 1→0 transitions). Cross-coverage of the staggered-cohort fit path is `tests/test_methodology_trop.py::TestTROPAlgorithm1LOOCV::test_control_set_includes_pretreat_of_eventually_treated` (two-cohort early-/late-treated panel under LOOCV-tuned `λ_unit`); absorbing-state structural validation is `tests/test_trop.py::TestDMatrixValidation`.
23912391
- [x] Special-case reductions (paper Section 2.2): **DID reduction verified** — TROP with `λ_nn=∞` + uniform weights matches TWFE-DiD ATT to within 0.5 of the analytical estimator on a TWFE-clean panel. **Matrix Completion code path exercised** — TROP with uniform weights + finite `λ_nn` engages the nuclear-norm prox solver (effective_rank > 0) and beats the DID-style baseline on a factor-confounded DGP; not an equivalence check against an independent MC reference. SC and SDID reductions are paper-claimed under "specific (omega, theta) weight choices" not provided in the paper text; cross-language anchor deferred until paper-author reference implementation clarifies the weight map. See `tests/test_methodology_trop.py::TestTROPSpecialCases`.
2392-
- **Note:** The balancing decomposition (paper Eq. 10) is a paper-side asymptotic identity. Direct numerical reconstruction of the four-term sum requires the internal `θ_s^{i,t}` / `ω_j^{i,t}` weight vectors, which are not exposed on the public TROP API; numerical Eq. 10 verification is therefore out of scope. The test `tests/test_methodology_trop.py::TestTROPNuclearNormProx::test_factor_matrix_consistent_with_treatment_effects` is a structural pointer (shape, finiteness, non-triviality of the fitted `factor_matrix`), NOT a full Eq. 10 lock. The Eq. 2 ingredients (soft-threshold SVD, **plain prox-gradient monotonicity** — NOT the shipped accelerated FISTA outer loop, which uses Nesterov momentum and does not guarantee per-step monotonicity, see `TestTROPNuclearNormProx` class docstring — weighted-prox) that the Eq. 10 derivation relies on are independently verified in the same class.
2392+
- **Note:** The balancing representation / decomposition (paper Eq. 10, Section 5.2) is a paper-side identity. Direct numerical reconstruction of the four-term sum requires the internal `θ_s^{i,t}` / `ω_j^{i,t}` weight vectors, which are not exposed on the public TROP API; numerical Eq. 10 verification is therefore out of scope. The test `tests/test_methodology_trop.py::TestTROPNuclearNormProx::test_factor_matrix_consistent_with_treatment_effects` is a structural pointer (shape, finiteness, non-triviality of the fitted `factor_matrix`), NOT a full Eq. 10 lock. The Eq. 2 ingredients (soft-threshold SVD, **plain prox-gradient monotonicity** — NOT the shipped accelerated FISTA outer loop, which uses Nesterov momentum and does not guarantee per-step monotonicity, see `TestTROPNuclearNormProx` class docstring — weighted-prox) that the Eq. 10 derivation relies on are independently verified in the same class.
23932393
- **Note (library-side choice):** Weight normalization (Gap #5 in `docs/methodology/papers/athey-2025-review.md`): paper Section 5 (p. 20) states weights sum to one (`1ᵀω = 1ᵀθ = 1`), but Eq. 3 (p. 7) writes unnormalized exponential weights. **The paper-side ambiguity remains open**; the library resolves it as a documented deviation — the shipped implementation matches Eq. 2 (unnormalized). Verified by `tests/test_methodology_trop.py::TestTROPDeviations::test_unnormalized_weights_match_eq2`. Will be revisited once paper-author reference implementation lands.
23942394
- **Note (deferral):** Equation 14 covariate extension (`Y_it = α_i + β_t + X_it·β_coef + R_it` with R low-rank, paper Section 6.2) is **not implemented**. `TROP.fit()` does not accept a `covariates` keyword argument. The corresponding Theorem 8.1 covariate-triple-robustness result is correspondingly out of scope. The non-support is locked by `tests/test_methodology_trop.py::TestTROPDeviations::test_covariates_not_supported`, which uses `inspect.signature` to guard against future `**kwargs` silently breaking the contract. Deferred until use cases motivate the X threading through `trop_local.py` / `trop_global.py` / LOOCV / bootstrap.
23952395
- **Note:** Survey support: weights, strata, PSU, and FPC are all supported via Rao-Wu rescaled bootstrap with cross-classified pseudo-strata (Phase 6). Rust backend remains pweight-only; full-design surveys fall back to the Python bootstrap path. Survey weights enter ATT aggregation only — population-weighted average of per-observation treatment effects. Model fitting (kernel weights, LOOCV, nuclear norm regularization) stays unchanged. Rust and Python bootstrap paths both support survey-weighted ATT in each iteration.

docs/methodology/papers/athey-2025-review.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
**PDF reviewed:** https://arxiv.org/abs/2508.21536v2 (version-pinned arXiv abstract for v2)
66
**Review date:** 2026-02-08
77

8+
**Version-pinning note (2026-05-25):** The current arXiv version of arXiv:2508.21536 is **v3**. The 2026-05-24 methodology promotion ships against this v2-pinned review; a formal v2-vs-v3 delta-check for TROP-relevant methodology changes (Eqs. 2-3, Algorithms 1-3, Section 2.2, Section 5.2-5.3, Section 6.1-6.2, Theorem 5.1, Corollary 1, Appendix Theorem 8.1) has NOT been performed. **Action item**: when the paper-author reference implementation lands (currently "forthcoming"), refresh this review against the most recent arXiv version and re-validate that the verified-component checklist still maps cleanly. Pending that refresh, the methodology promotion is anchored on v2 as documented here.
9+
810
---
911

1012
## Methodology Registry Entry

tests/test_methodology_trop.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
monotonicity, so the test exercises the plain prox-gradient
1616
ingredient (without momentum), not the accelerated loop.
1717
- Eq. 10: balancing decomposition of the estimated counterfactual is
18-
a paper-side asymptotic identity that depends on the internal
18+
a paper-side balancing representation (paper Eq. 10 / Section 5.2) that depends on the internal
1919
per-(i, t) weight vectors `theta_s^{i,t}` / `omega_j^{i,t}`,
2020
which are not exposed on the public TROP API. Direct
2121
numerical reconstruction of the four-term identity is out of
@@ -335,7 +335,7 @@ class TestTROPNuclearNormProx:
335335
The balancing decomposition of paper Eq. 10
336336
(``Y_NT_hat = L_NT + theta . (Y_pre_N - L_pre_N) + omega . (Y_T_co - L_T_co)
337337
- sum theta_t omega_i (Y_it_co - L_it_co)``)
338-
is a paper-side asymptotic identity that requires the internal per-(i, t)
338+
is a paper-side balancing representation (paper Eq. 10 / Section 5.2) that requires the internal per-(i, t)
339339
weight vectors ``theta_s^{i,t}`` / ``omega_j^{i,t}`` to numerically
340340
reconstruct. Those vectors are not exposed on the public TROP API,
341341
so this class does NOT directly verify the four-term identity. The

0 commit comments

Comments
 (0)