RobotLocomotion / drake

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

Improve code, documentation, exception messages, and testing for RotationMatrix handling of template type T symbolic::expression #14927

Closed mitiguy closed 2 weeks ago

mitiguy commented 3 years ago

For template type T symbolic expression, improve code, documentation, throwing better exception messages, and testing for the RotationMatrix class, e.g., methods involving conversion to RollPitchYaw or quaternion.

Background: While creating PR #14910, there was an exception thrown due to a template type T symbolic::expression in the method RollPitchYaw::CalcRollPitchYawFromQuaternionAndRotationMatrix(). This indicates that the class RollPitchYaw may need further testing for symbolic::expression.

SeanCurtis-TRI commented 3 years ago

One example where the code is unfriendly to symbolic::Expression can be found here:

https://github.com/RobotLocomotion/drake/blob/master/math/roll_pitch_yaw.cc#L141-L145

Rather than using if_then_else actual comparisons to M_PI require successful evaluation of the symbolic::Expression. This is not sole issue, merely the first one.

RussTedrake commented 3 years ago

I hit this myself earlier today. This was my simple reproduction (before finding this existing issue)

from pydrake.all import Variable, Expression, Quaternion_, RollPitchYaw_

w = Variable("w")
x = Variable("x")
y = Variable("y")
z = Variable("z")
quat = Quaternion_[Expression](w,x,y,z)
rpy = RollPitchYaw_[Expression](quat)

which results in

RuntimeError: The following environment does not have an entry for the variable w
jwnimmer-tri commented 2 weeks ago

For template type T symbolic expression, improve code, documentation, throwing better exception messages, and testing for the RotationMatrix class, e.g., methods involving conversion to RollPitchYaw or quaternion.

This is not an actionable statement. Closing as invalid.

I hit this myself earlier today. This was my simple reproduction (before finding this existing issue)

This example no longer crashes. Closing as fixed.