RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.22k stars 1.25k forks source link

Opaque Error Message For Quadratic Cost #20892

Open AlexandreAmice opened 6 months ago

AlexandreAmice commented 6 months ago

What happened?

Consider the following scenario.

1.) A user write a convex MathematicalProgram which include a quadratic cost. 2.) The user knows that the quadratic matrix Q is positive semidefinite and so uses the is_convex=True when calling AddQuadraticCost. 3.) Unbeknownst to the user, Q is numerically rank-deficient and so its smallest eigenvalue is near zero. Since is_convex=True, this is not detected when the constraint is passed to MathematicalProgram. 4.) The user calls Mosek (or some conic solver other than Clarabel) to solve their program.

The user will encounter the following very opaque error:

RuntimeError: Y is not positive semidefinite. 

What happened

Most conic solvers only support a linear costs. Therefore, when Solve is called, Drake will attempt to rewrite all the quadratic costs in epigraph from. This happens for example here. In order to do this, we call into math::DecomposePSDmatrixIntoXtransposeTimeX.

At this point, Solve() will return the error RuntimeError: Y is not positive semidefinite.

A user who knows nothing about the internal working of MathematicalProgram at this point is left confused. Even if one knows about the convex solvers, it is not clear which QuadraticCost is the culprit.

The fix

A better error message needs to be thrown when calling Solve(). Option 1) Add code in each solver which throws a better message (not preferred but maybe easier) Option 2) Add a method to QuadraticCost which allows it to rewrite itself and therefore throw a more convenient error message.

Notice that we still have the outstanding issue that DecomposePSDmatrixIntoXtransposeTimesX uses the Cholesky factorization which is unstable for low-rank matrices rather than the permuted LDLT factorization due to an outstanding Eigen bug.

@bernhardpg @nepfaff Since you originally poked me about this. If you guys have any simple MWE that'd be great.

Version

No response

What operating system are you using?

No response

What installation option are you using?

No response

Relevant log output

No response

jwnimmer-tri commented 6 months ago

I don't see too many calls from the solvers directory into DecomposePSDmatrixIntoXtransposeTimesX, so I don't think that adjusting all of those call sites is a big deal.

My first impression is that a good tactic to improve error reporting would be to add a function math::MaybeDecomposePSDmatrixIntoXtransposeTimesX that returns the error details instead of throwing. (The existing DecomposePSDmatrixIntoXtransposeTimesX would be a thin wrapper that calls Maybe... and then throws any errors.)

Then, change the solvers to call the Maybe... function instead and if they get an error, to report a more situationally-appropriate message.