fix(honest_did): nested optimal-FLCI optimizer — R center parity (SE-audit B2b)#635
Conversation
PR Review ReportOverall AssessmentExecutive Summary
MethodologyFinding 1 — P1: negative
|
igerber
force-pushed
the
fix/honest-flci-nested-parity
branch
from
f54041f to
4e6b78e
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR Review ReportOverall Assessment✅ Looks good — no unmitigated P0/P1 findings. Executive Summary
MethodologyFinding 1 — P3-informational: documented analytical critical-value deviationLocation: Impact: diff-diff uses an analytical folded-normal / folded noncentral-t critical value instead of R’s Monte Carlo Concrete fix: No action required. Finding 2 — P3-informational: documented signed-contrast bias conventionLocation: Impact: For signed Concrete fix: No action required. Prior Finding Check — resolvedNegative Code QualityNo findings. PerformanceNo findings. The SLSQP inner solve plus grid-zoom outer solve is documented as an implementation choice and covered by R parity tests. MaintainabilityNo findings. Tech DebtNo findings. The prior FLCI-center item is marked landed in SecurityNo findings. I did not see secrets or unsafe handling of external inputs in the changed files. Documentation/TestsFinding 3 — P3:
|
igerber
force-pushed
the
fix/honest-flci-nested-parity
branch
from
4e6b78e to
8fb12e0
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR Review ReportOverall Assessment✅ Looks good — no unmitigated P0/P1 findings. Executive Summary
MethodologyFinding 1 — P3-informational: documented analytical critical-value deviationLocation: Impact: diff-diff uses analytical folded-normal / folded noncentral-t critical values instead of R’s Monte Carlo Concrete fix: No action required. Finding 2 — P3-informational: documented signed-contrast bias conventionLocation: Impact: For signed Concrete fix: No action required. Prior Finding Check — resolvedNegative Locations: Code QualityNo findings. PerformanceNo findings. The SLSQP inner solve plus grid-zoom outer solve is a documented implementation choice for R center parity, not a methodology issue. MaintainabilityNo findings. Tech DebtNo blocker. The prior FLCI center-parity item is marked landed in SecurityNo findings. I did not see secrets or unsafe external-input handling in the changed files. Documentation/TestsFinding 3 — P3:
|
…audit B2b) Replace the flat Nelder-Mead over slope weights in the Delta^SD optimal FLCI with a faithful port of R HonestDiD::findOptimalFLCI's nested convex program: an inner minimum-worst-case-bias problem at a fixed estimator SD h (a smooth convex QCQP over the slope weights -- U >= |cumsum(w)|, one second-order-cone var(w) <= h^2, linear-trend neutrality; scipy SLSQP, no cvxpy, gated on feasibility not res.success) and an outer 1-D grid-zoom over h minimizing cv(M*bias/h)*h. The prior optimizer matched R's CI width but its center drifted up to ~9% from R's optimal affine estimator at intermediate smoothness M (the two land on different equi-width estimators). The nested structure removes that drift: center + half-length + optimalVec now match R to ~1e-3 (median ~1e-5) across a stress grid (num_pre x num_post x AR(1) rho x M). M=0 (min-variance estimator) is unchanged. The worst-case bias has the closed form M*(constant + sum_i|cumsum(w)_i|), verified equal to the LP oracle _compute_worst_case_bias to machine precision, so the inner objective is linear in the abs-linearized weights. diff-diff keeps its ANALYTICAL folded-normal critical value (_cv_alpha), strictly more accurate than R's Monte-Carlo .qfoldednormal; the golden overrides R's MC quantile with an analytical one so both solve the same deterministic outer problem (parity ~1e-3; vs stock MC-R ~1.4e-2, the residual being R's simulation noise on the near-flat width surface). A derivative/envelope-theorem outer solve is not viable -- the min-bias inner solutions are degenerate (most cumsum(w) at the abs-value kink), so the SD multiplier is not recoverable from sign(Lw). _compute_optimal_flci is now a thin (lb, ub) wrapper over _flci_solve (which also returns the optimal estimator direction for parity tests). Verified vs R HonestDiD 0.2.6 (benchmarks/data/honest_flci_golden.json + TestHonestFLCIParityR); 107 honest tests pass. Closes SE-audit B2b. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0178r8ZK6VRiXbaBiknrWjQB
igerber
force-pushed
the
fix/honest-flci-nested-parity
branch
from
8fb12e0 to
365e485
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR Review ReportOverall Assessment✅ Looks good — no unmitigated P0/P1 findings. Executive Summary
MethodologyFinding 1 — P3-informational: documented analytical critical-value deviationLocation: Impact: diff-diff uses analytical folded-normal / folded noncentral-t critical values instead of R’s Monte Carlo Concrete fix: No action required. Finding 2 — P3-informational: documented signed-contrast bias conventionLocation: Impact: For signed Concrete fix: No action required. Prior Finding Check — resolvedLocation: Negative Code QualityNo findings. The public PerformanceNo findings. The SLSQP inner solve plus grid-zoom outer solve is a documented implementation choice for R center parity, not a methodology issue. MaintainabilityNo findings. Tech DebtNo blocker. The prior HonestDiD FLCI center-parity item is marked landed in SecurityNo findings. I did not see secrets or unsafe external-input handling in the changed files. Documentation/TestsFinding 3 — P3-informational: local verification not executedLocation: test suite generally; new coverage at Impact: I could not run the targeted tests locally because importing the package fails with Concrete fix: Ensure CI runs the HonestDiD targeted tests, especially the R golden parity, zero-SD, signed-contrast, and negative- |
1 participant
Summary
HonestDiDΔ^SD optimal Fixed-Length CI (_compute_optimal_flci) with a faithful port of RHonestDiD::findOptimalFLCI's nested convex program: an inner minimum-worst-case-bias problem at a fixed estimator SDh(a smooth convex QCQP over slope weights —U ≥ |cumsum(w)|, one second-order-conevar(w) ≤ h², linear-trend neutrality; scipy SLSQP, no cvxpy, gated on feasibility notres.success) and an outer 1-D grid-zoom overhminimizingcv(M·bias/h)·h.M. The nested structure removes the drift — center + half-length + optimalVec now match R to ~1e-3 (median ~1e-5) across a stress grid (num_pre × num_post × AR(1) ρ × M).M=0(the min-variance estimator) is byte-unchanged.M·(constant + Σ_i|cumsum(w)_i|), linear in the abs-linearized weights (equals the LP oracle_compute_worst_case_biasfor nonnegativel_vec; for signed contrasts it follows R's conservative form — we match R either way, verified to ~1e-6)._cv_alpha), strictly more accurate than R's Monte-Carlo.qfoldednormal; the golden overrides R's MC quantile with an analytical one so both solve the same deterministic outer problem (~1e-3 parity; vs stock MC-R ~1.4e-2, the residual being R's simulation noise on the near-flat width surface)._compute_optimal_flciis now a thin(lb, ub)wrapper over_flci_solve(which also returns the optimal estimator direction for parity tests). Zero-SD / degenerate covariance returns NaN inference (no crash).Methodology references (required if estimator / math changes)
HonestDiDΔ^SD optimal Fixed-Length Confidence Interval (Rambachan & Roth 2023 §4.1); RHonestDiD::findOptimalFLCInested algorithm (.findWorstCaseBiasGivenHinner SOCP +.findOptimalCIDerivativeBisectionouter;.createObjectiveObjectForBiasclosed-form bias;.findLowestH/.findHForMinimumBiasbracket).HonestDiD0.2.6. Seedocs/methodology/REGISTRY.md(Δ^SD FLCI optimizer note)..qfoldednormal— strictly more accurate (noise-free); parity is asserted against override-R (deterministic) to ~1e-3, and against stock MC-R to ~1.4e-2 (R's own simulation noise). (2) For signed / contrastl_vecthe bias objective follows R's conservative closed form (matches R; the LP-oracle-exact bias would be tighter but would diverge from R). Both documented in REGISTRY.Validation
tests/test_methodology_honest_did.py— newTestHonestFLCIParityR(center + half-length + optimalVec vs R across the stress grid: override-R to 1e-3 + a stock-R MC-noise realism tier; a behavioral NaN test for inner-solve failure). Upgraded the M-grid test from finiteness-only to intermediate-M center parity; added zero-SD and signed-l_vecR-parity regression tests. New R goldenbenchmarks/data/honest_flci_golden.json(generatorbenchmarks/R/generate_honest_flci_golden.R, run from repo root), verified vs R HonestDiD 0.2.6. 107 honest tests pass;M=0and all existing behaviour unchanged.Security / privacy
🤖 Generated with Claude Code
https://claude.ai/code/session_0178r8ZK6VRiXbaBiknrWjQB