Skip to content

Remove LOOCV control observation subsampling from TROP#126

Merged
igerber merged 1 commit intomainfrom
trop-remove-subsampling
Feb 1, 2026
Merged

Remove LOOCV control observation subsampling from TROP#126
igerber merged 1 commit intomainfrom
trop-remove-subsampling

Conversation

@igerber
Copy link
Owner

@igerber igerber commented Feb 1, 2026

Summary

  • Remove max_loocv_samples parameter from TROP estimator — LOOCV now sums over ALL control observations per Equation 5 of Athey et al. (2025)
  • Parallelize inner LOOCV loops in Rust backend via Rayon fold/reduce to compensate for increased observation count
  • Delete test_joint_python_loocv_subsampling test and update all call sites

Methodology references (required if estimator / math changes)

  • Method name(s): TROP (Triply Robust Panel Estimator)
  • Paper / source link(s): Athey, S., Imbens, G. W., Qu, Z., & Viviano, D. (2025). Triply Robust Panel Estimators. https://arxiv.org/abs/2508.21536
  • Any intentional deviations from the source (and why): This PR removes a deviation. The previous max_loocv_samples subsampling was incorrectly attributed to the paper's footnote, but Footnote 2 only describes the two-stage coordinate descent search strategy. Equation 5 and Algorithms 1/2 explicitly sum over ALL control observations where W_it = 0.

Validation

  • Tests added/updated: tests/test_trop.py, tests/test_rust_backend.py
  • All 79 TROP tests pass (Rust backend: 80s, Python-only: ~49min)
  • All 62 Rust backend equivalence tests pass
  • LOOCV is now fully deterministic (no random subsampling)

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

The max_loocv_samples parameter (default 100) randomly subsampled control
observations before computing LOOCV scores. This was not specified in
Athey et al. (2025) — Equation 5 explicitly sums over ALL control
observations where D=0. Remove subsampling to match the paper exactly.

Inner LOOCV loops in the Rust backend are now parallelized via Rayon
fold/reduce to compensate for the increased observation count.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Feb 1, 2026

Overall assessment: ✅ Looks good
Executive Summary

  • TROP LOOCV now evaluates all control observations in both Python and Rust paths; aligns with the Methodology Registry’s Equation 5 requirements.
  • Rust LOOCV per‑observation loops are parallelized; note a minor performance/registry alignment concern around lack of short‑circuiting on failures.
  • Tests updated for new Rust LOOCV signatures and deterministic behavior; subsampling‑specific coverage removed.

Methodology

  • No findings. Affected method: TROP LOOCV tuning; registry Eq. 5 (sum over all D==0 controls; any fit failure ⇒ Q(λ)=∞) matches the implementation.

Code Quality

  • No findings.

Performance

  • P3 | Impact: For λ combinations with any failed fit, the new parallel loops still evaluate all controls before returning Q(λ)=∞, adding avoidable work on failure‑prone grids and diverging from the registry’s “early return” note. | Fix: Add early‑exit via try_fold/try_reduce + AtomicBool (or similar) to short‑circuit once a failure is detected, or update the registry wording if full evaluation is intended. (rust/src/trop.rs:L448-L495, rust/src/trop.rs:L1398-L1441)

Maintainability

  • No findings.

Tech Debt

  • No findings.

Security

  • No findings.

Documentation/Tests

  • No findings.

Open Questions

  • Assumption: Paper text not in repo; methodology check relies on docs/methodology/REGISTRY.md and in‑code docstrings.

Change Summary

  • Removed max_loocv_samples and subsampling; LOOCV uses all controls in Python and Rust paths.
  • Rust LOOCV per‑observation scoring is parallelized.
  • Tests updated for new Rust LOOCV signatures and deterministic behavior.

@igerber igerber merged commit 9985755 into main Feb 1, 2026
8 checks passed
@igerber igerber deleted the trop-remove-subsampling branch February 1, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant