fix(qp): align fast/cvxopt objective convention, reject dropped equalities, tune PDIPM - #372
Merged
Conversation
…ities, tune PDIPM
The registry documents min x'Hx + f'x, but the fast (PDIPM) and cvxopt
backends passed H straight into their native 0.5 x'Px + q'x form, so both
tracked 2*u_nom instead of u_nom (hidden in safety tests because the active
barrier pins the solution). Convert P = 2H at the registry boundary for
both; all four backends now agree on the audit probe and a new cross-solver
parity suite (interior, single-active, corner, random, and end-to-end
CBF-filter cases).
The fast backend also silently ignored a_mat/b_vec equality constraints, so
MPC through get_solver('fast') returned dynamics-violating trajectories with
success status; it now raises NotImplementedError pointing at jaxopt/casadi.
PDIPM: factor the reduced Newton matrix once per Mehrotra iteration
(predictor and corrector differ only in RHS), compute residuals once for
the convergence check and the step, and lower DEFAULT_MAX_ITER 25 -> 16.
16 was calibrated on 250 benign + 250 ill-conditioned near-boundary QPs:
deviation vs max_iter=60 is exactly 0.0 on both families (12 fails on 30/250
hard QPs precisely when the barrier is nearly violated; table in-file).
Measured 1.64x per solve. fori_loop retained: reverse-mode AD has no
while_loop rule and the differentiable-CBF-QP work grads through this
solver. The max_iter=25 pins in solve_qp_fast and fast_solver now defer to
the solver default.
test_fast_qp.py:112 expected [2,4], the registry-convention bug's answer;
the correct minimizer of x'x - 2x0 - 4x1 is [1,2] (2Hx + f = 0).
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.
Correctness
fastandcvxoptbackends solved the wrong objective. The registry convention ismin x'Hx + f'x, but both passedHstraight into their native0.5 x'Px + q'xform, tracking2·u_nominstead ofu_nom. Hidden in safety tests because the active barrier pins the solution. Fixed withP = 2Hat the registry boundary; all four backends now agree on the audit probe (u_nom=[1.0,-0.5]→[1.0,-0.5]) and on a new cross-solver parity suite (interior / single-active / corner / random / end-to-end CBF-filter cases).fastsilently dropped equality constraints, so MPC throughget_solver("fast")returned dynamics-violating trajectories with success status. Now raisesNotImplementedErrorpointing at jaxopt/casadi.test_fast_qp.pyexpected[2,4]— the bug's answer. The correct minimizer ofx'x − 2x₀ − 4x₁under this convention is[1,2].PDIPM performance (1.64× per solve, bit-identical results)
DEFAULT_MAX_ITER25 → 16, calibrated on 250 benign + 250 ill-conditioned near-boundary QPs: deviation vsmax_iter=60is exactly 0.0 on both families. (12 fails accuracy on 30/250 hard QPs precisely when the barrier is nearly violated — table inqp_solver_pdipm.py.)fori_loopretained deliberately:while_loophas no reverse-mode AD rule and the differentiable-CBF-QP work takes gradients through this solver. Themax_iter=25pins insolve_qp_fast/fast_solvernow defer to the solver default.Verification
jax.gradthrough the solver verified finite.Merge order: this is PR 1 of a 4-PR stack (merge top-down).