Add a top-level generic "solver" option with a Frank-Wolfe solver for the SCM weights - #117
Open
paulgp wants to merge 1 commit into
Open
Add a top-level generic "solver" option with a Frank-Wolfe solver for the SCM weights#117paulgp wants to merge 1 commit into
paulgp wants to merge 1 commit into
Conversation
… the SCM weights `solver = "osqp"` (default) is the existing behavior, with synth_qp's body moved unchanged to R/solvers.R. Solvers are plain functions (X1, X0, V) -> weights resolved by get_synth_solver(), and `solver` also accepts such a function directly, so new solvers can be plugged in without touching the package. `solver = "frank_wolfe"` runs Frank-Wolfe with exact line search (the synthdid algorithm; Arkhangelsky et al. 2021, dual BSD-3/GPL>=2) to identify the active donor set, solves the QP exactly on that support (OSQP on a problem of at most 5*t0 donors), and verifies KKT optimality with a full gradient screen that re-admits missed donors. The n0 x n0 donor Gram matrix is never formed, so memory stays O(n0 t0): at 20,000 donors the Gram alone is 3.2 GB while the Frank-Wolfe path runs in ~5s. The option threads through everywhere single-treated SCM weights are solved (fit_synth_formatted, the ridge base weights, the ridge lambda CV refits, and fit_augsyn), lands in extra_args so inference / permutation / cv refits reuse it, and is inherited by augsynth_multiout; multisynth is untouched and warns that the option is ignored. tests/testthat/test_solvers.R checks solver agreement on the raw QP, SCM with and without ridge augmentation (fixed and CV lambda), with a fixed effect, the custom-function hook, and that refits remember the solver. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4N6wSPuTLbSVDtsnEafKT
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following up on our email exchange — and to answer your question directly: yes, it's memory (and time) with large donor pools.
synth_qp()forms the dense n0 x n0 donor Gram matrix before handing it to OSQP: 200 MB at 5,000 donors, 3.2 GB at 20,000. In our financial applications (S&P index events against an all-CRSP donor pool) each event has 4,000–7,000 donors and there are hundreds of events, so the Gram construction dominates everything.What this PR does
1. A top-level, generic
solveroption (your first ask).single_augsynth()gainssolver = "osqp"— the default is the existing behavior, with the currentsynth_qpbody moved verbatim toR/solvers.Rassynth_qp_osqp(). Solvers are plain functions with signature(X1, X0, V) -> weights, resolved byget_synth_solver();solveralso accepts such a function directly, so anyone can plug in a different solver without touching the package:The option threads through every place single-treated SCM weights are solved —
fit_synth_formatted(), the ridge base weights, the ridge lambda CV refits (cv_lambda/get_lambda_errors), andfit_augsyn()— and it lands inextra_args, so the jackknife/conformal/permutation/cv refits automatically reuse it.augsynth_multioutinherits it throughfit_augsynth_internal.multisynthis untouched, and theaugsynth()dispatcher warns thatsolveris ignored there (mirroring the existingprogfuncwarning), per your note about the bespoke staggered-adoption optimization.2. The
"frank_wolfe"solver. Frank–Wolfe with exact line search (the synthdid algorithm — Arkhangelsky, Athey, Hirshberg, Imbens & Wager 2021, dual BSD-3/GPL≥2; implementation written for this PR) to identify the sparse active donor set, then the QP solved exactly on that support (OSQP on a problem of at most 5·t0 donors), plus a KKT gradient screen that re-admits any donors the support restriction missed. Pure FW alone has a sublinear tail and is not accurate enough — the support-restricted polish is what makes the two solvers agree tightly. The full Gram matrix is never formed, so memory stays O(n0·t0):3. Unit tests (your second ask).
tests/testthat/test_solvers.Rchecks, on the basque fixture and a simulated QP: solver agreement on the raw QP objective; SCM without ridge augmentation (imbalance and ATT path); ridge augmentation with a fixed lambda; ridge with CV lambda (the CV refits run through the requested solver and select the same lambda); afixedeff = TRUEvariant; that a custom solver function reproduces the default exactly; and that inference refits remember the solver viaextra_args. All pass.Test status
On my machine a fresh clone of current master has 19 failing tests (dependency-version drift / missing optional Suggests, none related to this area). I ran the full suite against stock master and against this branch into separate libraries and diffed the failing sets: they are identical — this PR introduces zero new failures and its 16 new assertions all pass.
Default behavior is unchanged:
solver = "osqp"runs the exact code that ran before.🤖 Generated with Claude Code
https://claude.ai/code/session_01S4N6wSPuTLbSVDtsnEafKT