RobotLocomotion / drake

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

Not particularly useful MbP error on reaction forces computation #15720

Closed edrumwri closed 2 years ago

edrumwri commented 3 years ago

abort: Failure at multibody/plant/tamsi_solver.cc:217 in SolveQuadraticForTheSmallestPositiveRoot(): condition 'Delta > 0' failed

I don't have any public code I can share to reproduce this, but it happens on https://github.com/RobotLocomotion/drake/commit/3d8c50818cce30c735d60bb56de91724ab5437f6 when the actuation input port has a NaN value and I request the reaction forces from the appropriate MBP output port.

rpoyner-tri commented 3 years ago

What should the correct behavior be when NaN appears on the input? NaNs out? error with a friendlier message?

sherm1 commented 3 years ago

My preference would be an error message like "Actuation input was NaN."

amcastro-tri commented 3 years ago

would that be an MBP-specific message or more of a system's framework feature? Maybe the check should be turned off in release builds?

sherm1 commented 3 years ago

If we add a check for NaN I think it should be system-specific rather than a framework feature. It isn't necessarily an error to have NaNs flow through ports. For example, an adder can take NaN inputs and reasonably produce a NaN output (as would happen in code). But MBP doesn't like it when a NaN is provided as an actuation force.

xuchenhan-tri commented 2 years ago

From the discussion above, it seems like people are ok with emitting an error message when a NaN shows up in a MbP input port? FWIW, personally, I think that's the right fix for this issue.

amcastro-tri commented 2 years ago

I agree on the solution.

xuchenhan-tri commented 2 years ago

Thanks for the input. I'll work on this one in the coming weeks.