Skip to content
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 a tutorial on the debugging tips for MathematicalProgram. #14815

Merged
merged 1 commit into from Mar 25, 2021

Conversation

hongkai-dai
Copy link
Contributor

@hongkai-dai hongkai-dai commented Mar 24, 2021

Resolves #13091

This change is Reviewable

@hongkai-dai hongkai-dai force-pushed the tutorial_debug_mp branch 2 times, most recently from f3c1704 to 02bca58 Compare March 24, 2021 01:18
Copy link
Contributor Author

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+@RussTedrake for feature review please, thanks!

Reviewable status: LGTM missing from assignee RussTedrake(platform), needs at least two assigned reviewers (waiting on @RussTedrake)

Copy link
Contributor

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Minor comments below.!

Reviewed 5 of 5 files at r1.
Reviewable status: 7 unresolved discussions, LGTM missing from assignee RussTedrake(platform), needs at least two assigned reviewers (waiting on @hongkai-dai)


tutorials/debug_mathematical_program.ipynb, line 43 at r1 (raw file):

    "def constraint(x):\n",
    "    return [np.cos(x[0]) + 2 * np.cos(x[0] - x[1])]\n",
    "# Find a solution satisfying\n",

nit: add a whitespace line here?


tutorials/debug_mathematical_program.ipynb, line 44 at r1 (raw file):

    "    return [np.cos(x[0]) + 2 * np.cos(x[0] - x[1])]\n",
    "# Find a solution satisfying\n",
    "# 1 <= cos(x[0]) + 2 * cos(x[0] - x[1]) <= 2\n",

perhaps you could even plot this landscape here? it would probably help...


tutorials/debug_mathematical_program.ipynb, line 69 at r1 (raw file):

   "metadata": {},
   "source": [
    "In the example above, you could see that with a bad initial guess $x = [0, 0]$, the solver gets stuck and reports the problem being infeasible (the reason for getting stuck is that the gradient of the constraint function $cos(x[0]) + 2cos(x[0]-x[1])$ is zero at the initial guess $x=[0, 0]$, hence the gradient-based solver doesn't know how to move the decision variables. This phenominon also appears when solving an inverse-kinematics problem with an initial pose at singularity, or solving a unit-length constraint $x^Tx=1$ with initial guess $x=0$); but by changing the initial guess, the solver can find a solution.\n",

typo: should be "phenomenon"


tutorials/debug_mathematical_program.ipynb, line 165 at r1 (raw file):

    "constraint1 = prog.AddConstraint(lambda z: [z.dot(z)], [1], [np.inf], x)\n",
    "constraint1.evaluator().set_description(\"outside unit circle\")\n",
    "# Add the constraint x[0]**2 + 4 * x[1]**2 <= 4\n",

nit: this is a wall of text/code. add some whitespace lines?


tutorials/debug_mathematical_program.ipynb, line 259 at r1 (raw file):

    "ax.axis('equal')\n",
    "\n",
    "def update(x):\n",

can we choose a better name for this function? even "visualization_callback"?


tutorials/debug_mathematical_program.ipynb, line 301 at r1 (raw file):

    "p1 = prog.NewContinuousVariables(2, \"p1\")\n",
    "p2 = prog.NewContinuousVariables(2, \"p2\")\n",
    "# Add the constraint that p1 is in an ellipsoid (p1(0)-1)**2 + 4*(p1(1)-2)**2 <= 1\n",

nit: whitespace


tutorials/debug_mathematical_program.ipynb, line 335 at r1 (raw file):

    "prog = mp.MathematicalProgram()\n",
    "x = prog.NewContinuousVariables(2)\n",
    "# Add the constraint x^T * x <= 1\n",

nit: whitespace

@RussTedrake
Copy link
Contributor


tutorials/debug_mathematical_program.ipynb, line 203 at r1 (raw file):

    "4. For Gurobi, call `prog.SetSolverOption(GurobiSolver().solver_id(), \"OutputFlag\", True)`.\n",
    "5. For SCS, call `prog.SetSolverOption(ScsSolver().solver_id(), \"verbose\", 1)`.\n",
    "6. For OSQP, call `prog.SetSolverOption(OsqpSolver().solver_id(), \"verbose\", 1)`."

anything for MixedIntegerBranchAndBound?

Copy link
Contributor Author

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 4 unresolved discussions, LGTM missing from assignee RussTedrake(platform), needs at least two assigned reviewers (waiting on @RussTedrake)


tutorials/debug_mathematical_program.ipynb, line 44 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

perhaps you could even plot this landscape here? it would probably help...

Done. Good call.


tutorials/debug_mathematical_program.ipynb, line 69 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

typo: should be "phenomenon"

Done.


tutorials/debug_mathematical_program.ipynb, line 203 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

anything for MixedIntegerBranchAndBound?

Good call. Currently I haven't implemented anything print out method for the MixedIntegerBranchAndBound class. I will add that later. I filed the issue #14822


tutorials/debug_mathematical_program.ipynb, line 259 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

can we choose a better name for this function? even "visualization_callback"?

Done.

Copy link
Contributor

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: thanks for doing this!

Reviewed 1 of 1 files at r2.
Reviewable status: needs at least two assigned reviewers (waiting on @hongkai-dai)

Copy link
Contributor Author

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+@ggould-tri for platform review please, thanks!

Reviewable status: LGTM missing from assignee ggould-tri(platform) (waiting on @ggould-tri)

Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: ; one note below.

FYI I'm hoping to bring in Anzu's collision remover code soon, so being able to reference this tutorial in the documentation when I do so will make my life much easier!

Reviewed 1 of 5 files at r1, 1 of 1 files at r2.
Reviewable status: 1 unresolved discussion (waiting on @hongkai-dai)


tutorials/debug_mathematical_program.ipynb, line 25 at r2 (raw file):

    "After constructing and solving an optimization problem through Drake's `MathematicalProgram` interface, you might not get the desired results. For example, you might expect the problem to have a solution, while `MathematicalProgram` reports that the problem is not solved succesfully. In this tutorial we provide some tips to debug `MathematicalProgram` when it doesn't behave desirably.\n",
    "\n",
    "First you should understand whether the optimization problem is convex or not. For a convex problem (like LP, QP, SDP), when the problem is feasible, then theoretically the solver should always find a solution; on the other hand if the problem is non-convex and solved through gradient-based solvers (like SNOPT/IPOPT), then the solver might fail to find a solution even if one exists. When the gradient-based solver (like SNOPT/IPOPT) reports the problem being infeasible, it only means that locally in the search region, the optimization problem doesn't have a solution. A solution could exist far away from where the solver gets stuck, but the solver is trapped in a certain region and can't jump to the distant solution. Often if you improve the initial guess, then the solver might be able to find the solution.\n",

This isn't really a correct description of SNOPT, which can report infeasibility even if the initial guess was feasible or it has reached feasible points in the search if its most recent guess is infeasible when it hits its iteration limit.

Copy link
Contributor Author

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees ggould-tri(platform),RussTedrake(platform) (waiting on @ggould-tri and @RussTedrake)


tutorials/debug_mathematical_program.ipynb, line 25 at r2 (raw file):

Previously, ggould-tri wrote…

This isn't really a correct description of SNOPT, which can report infeasibility even if the initial guess was feasible or it has reached feasible points in the search if its most recent guess is infeasible when it hits its iteration limit.

Done, good call. I added a paragraph on SNOPT to explain that it could jump to an infeasible solution from a feasible one.

Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees ggould-tri(platform),RussTedrake(platform) (waiting on @hongkai-dai)

@hongkai-dai hongkai-dai merged commit 1768921 into RobotLocomotion:master Mar 25, 2021
@hongkai-dai hongkai-dai deleted the tutorial_debug_mp branch July 13, 2023 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mathematicalprogram: add constraint naming/debugging workflow to tutorials
3 participants