RobotLocomotion / drake

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

Unify mass properties defaults for a better user experience #17446

Closed sherm1 closed 8 months ago

sherm1 commented 2 years ago

There are four ways to specify a link (body) in Drake today: urdf, sdformat, mujoco, or programmatic. If the mass properties are left unspecified, each of these four has a different default:

method default mass default inertia
urdf 0 0 0 0
sdformat 1 1 1 1
mujoco (no geometry) 0 0 0 0
mujoco (geometry) estimated estimated
programmatic NaN NaN NaN NaN

Proposal

Adopt the sdformat default uniformly (except for the "estimated" case).

What problems does this solve?

Issue #17081 asked for better error messages in the case of a user inappropriately using massless bodies. However, currently we can't distinguish unspecified masses from bad zero masses because in some cases (urdf and mujoco/no geom) the zeros just indicate no mass props were specified, possibly because the user only intends to do kinematics. If we complain about those in Finalize(), users who had no intention of doing dynamics will be annoyed. Note that changing the default to sdformat's still leaves kinematics working fine, and is no worse for dynamics than zeros and identical to what would happen if the same model had been input through an sdformat file.

With that change we can confidently detect non-physical use of explicitly-defined massless bodies at Finalize() and provide a helpful error, without annoying users who had no intention of using mass at all.

Note that urdf wasn't originally intended for simulation, hence the same people developed sdf (simulation description format) to meet that need. They chose the non-zero default after substantial experience trying to do simulations with urdf files!

For us switching from zeros and NaNs to ones will eliminate most of the awful delta > 0 messages in exchange for systems that may exhibit wrong behavior due to having wrong mass properties. That is a standard system ID problem though and likely much easier for a user to deal with than the obscure solver failure message. Documentation for a uniform default will be easy to state and easy to understand. We could provide a Drake-specific tag to revert to the usual defaults but I don't think we should do that unless there is a present & compelling use case.

Looking for input from real Drake users and people who see their troubles. Is this a good idea?

cc @RussTedrake @jwnimmer-tri @rcory @ggould-tri @sammy-tri and anyone else with an opinion

jwnimmer-tri commented 2 years ago

Replaying some of my feedback from a f2f meeting with Sherm on this yesterday...

Before we haul off and change the URDF default in our parser, we should survey some of the existing corpus of URDF robot models to see whether adding more mass would break existing users. Perhaps some of them use zero-mass welded links on purpose (for convenient relative posturing, or similar reasons), and they might do that by using the default-implicit zero mass. We might still decide to break those users for a greater good, but we should at least be informed while discussing that trade-off.

RussTedrake commented 2 years ago

One problem I foresee with the sdformat default is that people often make false joints - eg three prismatic and three rotary joints with zero mass - in order to add actuators to a floating base. These are typically done without adding any inertial attributes. You will break these urdfs.

The mujoco defaults handle that situation nicely.

sherm1 commented 2 years ago

I would think that creating joints with massless intermediaries is an advanced use case? Maybe those users could be instructed to explicitly designate massless bodies when they want them. The tradeoff would be that we could eliminate lots of delta > 0 errors for beginning users (in exchange for "successful" runs with incorrect mass properties). We would also be able to catch incorrect use of explicitly-zero massless bodies (e.g. terminal ones) without disturbing kinematics-only users (who presumably won't have provided inertial properties).

RussTedrake commented 2 years ago

Changing even an advanced user's existing, working urdf, with zero-mass internal links, to suddenly have mass seems like a bad outcome. And "all ones" seems like a pretty nonsensical default to me.

I still think that the mujoco defaults are quite good -- I think most users will have geometry associated with the links they care about, and would not be surprised by having zero-mass on links with no geometry.

ggould-tri commented 2 years ago

FWIW, this would break the old "floating gripper" synthesis work downstream. We no longer use that code, so it's a not a big cost, but if you were looking for a concrete use case that would be it.

I assume from the fact that we're contemplating this that just handling those zero-mass linking bodies in dynamics is too difficult to be considered? If that's the case, I think that this proposal's enabling readable error messages (ie, I assume, messages that name the offending link and the specific objectionable properties) is really the best outcome; only the user knows what the correct mass for a nonexistent, nonphysical shim is, and the better we inform them the better.

sherm1 commented 2 years ago

The zero-mass shims are fine and don't cause any problems. Also zero-mass links welded to massful links are fine too. We were hoping to be able to issue fail-fast error messages for zero-mass links that will definitely cause a problem, such as a zero-inertia end effector connected with a revolute joint. But ... those don't cause problems for users doing only kinematics who left mass props unspecified, so we can't fail on zero mass terminal bodies if zero is the default.

From the above comments I infer that we will not be able to give early error messages at all, but will have to wait until we know for sure dynamics is being performed. For now we can provide a function like IsOkForDynamics() that can perform the checks and attempt to point out the culprit. I'm not sure yet when we could invoke that automatically.

jwnimmer-tri commented 8 months ago

I don't think we should change the semantics of existing file formats.

Possibly we should change what happens with "programmatic" bodies, but we can do that on its own merits instead of a "unify across all formats" epic.