-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add dual solution to OsqpSolverDetails. #12916
Add dual solution to OsqpSolverDetails. #12916
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+@EricCousineau-TRI for both reviews please, thanks!
Reviewable status: LGTM missing from assignee EricCousineau-TRI(platform), needs at least two assigned reviewers (waiting on @EricCousineau-TRI)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 5 files at r1.
Reviewable status: 1 unresolved discussion (waiting on @hongkai-dai)
solvers/osqp_solver.h, line 33 at r1 (raw file):
// y contains the solution for the Lagrangian multiplier associated with // l <= Ax <= u. The Lagrangian multiplier is set only when OSQP solves // the problem. Notice that the order of the linear constraints are linear
nit Can you make this a docstring? ///
solvers/test/osqp_solver_test.cc, line 130 at r1 (raw file):
EXPECT_EQ(result.get_solver_details<OsqpSolver>().status_val, OSQP_SOLVED); EXPECT_TRUE(CompareMatrices(result.get_solver_details<OsqpSolver>().y, Eigen::Vector3d(0, 0, -0.0619621), 1E-4));
BTW This tolerance seems high?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all discussions resolved, LGTM from assignee EricCousineau-TRI(platform)
solvers/test/osqp_solver_test.cc, line 130 at r1 (raw file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
BTW This tolerance seems high?
Done. OSQP is not very accurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2.
Reviewable status: commits need curation (https://drake.mit.edu/reviewable.html#curated-commits)
cc @pangtao22
This change is