Document behaviour changes introduced #1784

[arjo129]

🦟 Bug fix

Summary

The PR in #1784 claimed not to introduce behavior changes, but actually ended up introducing behavior changes. Specifically, it would break a lot of marine simulation code that runs on our tools. The issue in question is a matter of naming parameters. In the past <xUU> used to refer to the quadratic drag term in the diagonal axis. This term would be multiplied by the absolute velocity. After #1784 this term was multiplied by the velocity (no absolute). An equivalent term of <xUabsU> This behavior change likely breaks all previous maritime simulations that rely on our hydrodynamics plugin.

There are several options:

1. Revert Adds support for hydrodynamic cross terms #1784
2. Make <xUU> mirror <xUabsU>
3. Document the change and mark it as breaking.

This PR goes with option 3 as there are already code bases using <xUabsU> nota tion instead of <xUU>. We also add a warning if someone does use the <xUU> term that points to this PR for details.

Checklist

• Signed all commits for DCO
• Added tests
• 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.

[Sleipnir164]

Thank you, I changed xUU to xUabsU , same for y and z and everything behaves as it used to!

[ejalaa12]

This is an important and good thing to document quickly.
I had to find (through the source code) that the new parameter to use are instead of .

This also begs the question: in which scenario would you need ? Basically, it would make the drag dependent on the direction of the movement. For example, in a forward movement, the drag slows down the link, but in a backward movement it would accelerate it. Which is weird, no?

Shouldn't the quadratic term always be multiplied the absolute velocity?

[arjo129]

@quarkytale @mabelzhang pinging again for ✔️ . We also should investigate why the tests didn't catch this (that can be a separate PR). To test simply run the example AUV world.

[quarkytale]

I think tests aren't accounting for the range of weight typically the use-cases have. Lower weights might be preventing large translations.

[arjo129]

Lower weights might be preventing large translations

Doesn't quite make sense. Since F = ma won't smaller m lead to bigger a?

This also begs the question: in which scenario would you need ? Basically, it would make the drag dependent on the direction of the movement. For example, in a forward movement, the drag slows down the link, but in a backward movement it would accelerate it. Which is weird, no?

For diagonal terms this is true. Cross terms add forces due to motion in one axis along another axis.

[quarkytale]

Well you are right. Refining that, the range of added mass and drag terms. The sim crashed for me when I had those values in range of 10^3 and more, less than that kept the sim running (but incorrect behavior of accelerating in backward motion). In the test's world we should probably have another model with higher values.

[quarkytale]

LGTM after minor formatting

[arjo129]

BTW Might be worth adding an assertion on the following line to check direction of motion.

gz-sim/test/integration/hydrodynamics.cc

Line 136 in c03c432

EXPECT_LE(sphere1Vels[i].Z(), sphere2Vels[i].Z());