Fluid added mass / symmetric matrix

[chapulina]

🎉 New feature

• Part of Hydrodynamic added mass gz-sim#1462
• Alternative to Add MatrixX class #442

Summary

Based on the feedback on #442, I started implementing a specific class to hold fluid added mass. I stopped halfway through the implementation because various aspects of this didn't feel right to me, so I'm opening this for others to take a look and we have something more concrete to discuss.

I couldn't come up with ways to make the API a bit more semantically specific to added mass, and I don't even know if we want to. Maritime literature usually uses numeric indices to denote these terms within the matrix and AFAICT it is not common to decompose the matrix. It's true that we may need to shift submatrices around to adapt to different conventions, such as swapped rotational and translational components, or even shift things even more if a physics engine doesn't use Z-up. But I feel like this is the kind of thing that we can address with documentation, not APIs.

So my inclination at this point is to go with either:

• The MatrixX class in Add MatrixX class #442
• This PR, with FluidAddedMass renamed to SymmetricMatrixX and made general since there isn't anything specific to "6" in the implementation so far.

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

[scpeters]

I'd like fluid added mass to be in its own class, but I'd also like there to be a separate Inertial class that uses it, so that math::Inertial only supports rigid body inertial terms, while the more advanced Inertial class could also support fluid added mass.

I sketched out how the API could look in 43aa14b, placing the new classes in the eigen3 component and using Eigen::Matrix<T, 6, 6> for the 6x6 spatial inertia matrices. The naming would need work (math::Inertial and math::eigen3::Inertial would probably be confusing), but this is how I would envision representing added mass.

[chapulina]

a separate Inertial class that uses it

Can you elaborate the motivation? What's the downside of having the added mass in the existing inertial class? The added mass doesn't affect the rigid body terms unless it is set, so it is a backwards-compatible addition. How do you expect the two inertial classes to interact? Why have two classes that duplicate inertial information?

Are you proposing we also separate it on the SDF spec? It sounded like you were ok keeping it within <inertial> on gazebosim/sdf_tutorials#76. So it feels odd to have it all working together on SDF, but not on Gazebo Math.

And how do you expect downstream libs like gz-physics and gz-sim to handle these 2 different inertial classes?

[scpeters]

a separate Inertial class that uses it

Can you elaborate the motivation? What's the downside of having the added mass in the existing inertial class? The added mass doesn't affect the rigid body terms unless it is set, so it is a backwards-compatible addition. How do you expect the two inertial classes to interact? Why have two classes that duplicate inertial information?

The existing Inertial class currently supports only rigid body inertial properties, and there are many robotic applications that don't require fluid added mass parameters, so I think it is useful to be have a class that is conceptually focused on that application. Not all physics engines support a spatial inertia matrix (there are even questions about dart's support gazebosim/sdf_tutorials#76 (comment)), so I think being able to use distinct data types for Inertials with or without fluid added mass in a gz-physics Feature API would be most clear.

Are you proposing we also separate it on the SDF spec? It sounded like you were ok keeping it within <inertial> on gazebosim/sdf_tutorials#76. So it feels odd to have it all working together on SDF, but not on Gazebo Math.

And how do you expect downstream libs like gz-physics and gz-sim to handle these 2 different inertial classes?

I'm fine with keeping it all under <inertial> in SDFormat, and in 43aa14b, I've proposed the larger inertial class to contain a math::Inertial object alongside the FluidAddedMass object. I would expect gz-sim to prefer the Inertial class with fluid added mass. I would expect libsdformat to provide both types of Inertial objects from the Link API. I would expect gz-physics to have separate Features for supporting Inertials with and without fluid added mass.

[chapulina]

I would expect gz-physics to have separate Features for supporting Inertials with and without fluid added mass.

Agreed, this makes sense. But we don't need to split the Inertial class to achieve this, right?

I would expect libsdformat to provide both types of Inertial objects from the Link API.

In general, we try to mimic the SDF spec with our C++ classes. This trickles down to components in gz-sim, messages in gz-msgs, widgets in the UI. Your proposal would be a divergence from that pattern.

This would also force downstream packages to make a decision at the API level about which type to use, and possibly having to handle both. A user can always check if fluid mass is set or not, instead of having to deal with two separate types.

I would expect gz-sim to prefer the Inertial class with fluid added mass.

Who would be the users of the inertial class without added mass? Bare in mind that gz-physics can have separate APIs for handling inertia with added mass and without, and both APIs can take in the same object.

It's similar to how the sdf::Joint class has the ThreadPitch function, but that's only relevant to screw joints.

[chapulina]

Per #442 (comment), I'll be opening a new PR with a Matrix6 class that follows the Matrix[3|4] pattern.