Open
Conversation
…ests The regularized KKT solution g satisfies (KKT)g = [c; -b], so -g solves with RHS [-c; b] and approximates (x*, y*) as R -> 0. The x default was using g[i] (wrong sign) instead of -g[i]; fixed in both warm_start_vars and cold_start_vars. Added partial_warm_start_qafiro test (n=32, m=60) exercising various partial initialization patterns: x-only, half-x, x+y, x+s, y-only, and perturbed x+y at 1% and 10%. Key result: x+y with NaN s converges in 0 iters (direct) since s = b - Ax recovers optimal s perfectly.
Member
Author
|
In my experiments, constructing s from x and y is a huge boost when x and y are perturbed by 10%. |
The strict < assertion was failing in CI where y-only warm start matched cold start iteration count exactly.
NaN out first 2 entries of x and y instead of testing y-only, which had flaky iteration count comparisons across platforms.
Member
Author
|
CI has me thinking that maybe this is only an improvement on Linux! I'll debug this more tomorrow. |
The indirect solver sometimes matches cold start iteration count exactly. Keep the strict < assertion for the direct solver.
- Remove x+s (NaN y) iteration assertion: can be worse than cold start on some platforms due to inconsistent starting point - Relax perturbed x+y assertions to <= for indirect solver - Keep strict < assertions for direct solver where improvement is reliable
The indirect solver's iteration counts are too variable across platforms for reliable warm start comparisons. Only check convergence for indirect; keep strict assertions for direct.
Member
|
Cool, thanks for doing this! I will patch it in and see how the performance changes on a test suite I have and see if it helps, and if so we can land this. |
Member
|
Seems roughly neutral right now, it's an average saving of about 1% in terms of iterations on the instances of Maros-Meszaros that both versions (this PR vs HEAD) solved, but solved one fewer. Maybe there is some other tweak we can do to accelerate it in the initialization? 
|
Member
|
Per instance log-iterations: |
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
2 participants
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.
Hey Brendan!
In cvxpy/cvxpy#3123 and cvxpy/cvxpy#3124, we're experimenting with more intelligent warm-starting of solvers. One issue is the solver has to support NaN in the warm start. This makes SCS a perfect choice since you've supported NaN warmstarts already.
I, however, was poking through the code to make sure that you had the correct support for NaN warmstarts and was a bit surprised that you were 0 filling the initial vectors when with a little reordering in update_work, we could instead initialize the vectors to the solution to the linear solve with RHS = [c ; -b]. This seemed possibly better to me, as it represents an initial point that is the solution to the regularized KKT conditions for the all equality case, and then s selected so that x and s are consistent. Intuitively, this made more sense as it should have all the initial points at the right orders of magnitude.
I'm curious what you think of this change. Happy to trash it if I didn't take Lemma 5.1 into account from your paper!
Additionally, I had Claude generate a test file to verify that partial warm starting is working. I hope that the test files are useful even if I'm messing up the operator splitting in suggesting an initialization change.