Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update added-mass of lrauv model and remove added mass definitions from examples #2608

Open
wants to merge 9 commits into
base: gz-sim9
Choose a base branch
from

Conversation

arjo129
Copy link
Contributor

@arjo129 arjo129 commented Sep 9, 2024

🦟 Bug fix

Fixes #2575

Summary

Since, we have decided to deprecate added mass in the hydrodynamics plugin in favor of using the SDFormat fluid_added_mass tag (See #2493 ), we should update our examples to avoid the warning.

TODO: Update maritime tutorials and migration guide.

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.

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

examples

Fixes #2575

Since, we have decided to deprecate added mass in the hydrodynamics
plugin in favour of Rigid Bodies, we should update our
examples to avoid the warning.

TODO: Update maritime tutorials and migration guide.

Signed-off-by: Arjo Chakravarty <[email protected]>
@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Sep 9, 2024
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
I'm reverting DVL tests as they never considered added mass. Adding
added mass changes the expected velocities.

Signed-off-by: Arjo Chakravarty <[email protected]>
@arjo129 arjo129 marked this pull request as ready for review September 17, 2024 00:06
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put all of the Fuel models in the openrobotics account?

test/worlds/bottomless_pit.sdf Outdated Show resolved Hide resolved
Copy link

codecov bot commented Oct 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.76%. Comparing base (1d6651d) to head (dfee475).
Report is 42 commits behind head on gz-sim9.

Additional details and impacted files
@@             Coverage Diff             @@
##           gz-sim9    #2608      +/-   ##
===========================================
+ Coverage    68.73%   68.76%   +0.02%     
===========================================
  Files          341      341              
  Lines        33011    33049      +38     
===========================================
+ Hits         22690    22726      +36     
- Misses       10321    10323       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

# Migrating Added Mass from the Hydrodynamics to the Fluid Added Mass tag

Historically, we used the specify added mass in the parameters of the hydrodynamics plugin.
Starting with GazeboIonic, we have deprecated that in favour of using the SDF `<fluid_added_mass>` tag directly. This guarantees better numerical stability. That being said, in order to actually simulate added mass your underlying physics engine must be able to support the added mass tag. So far that only includes the DART physics engine.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Starting with GazeboIonic, we have deprecated that in favour of using the SDF `<fluid_added_mass>` tag directly. This guarantees better numerical stability. That being said, in order to actually simulate added mass your underlying physics engine must be able to support the added mass tag. So far that only includes the DART physics engine.
Starting with Gazebo Ionic, we have deprecated that in favour of using the SDF `<fluid_added_mass>` tag directly. This guarantees better numerical stability. That being said, in order to actually simulate added mass your underlying physics engine must be able to support the added mass tag. So far that only includes the DART physics engine.

@@ -271,16 +271,10 @@
</plugin>

<!-- Uncomment to add hydrodynamics -->
<!-- <plugin
<plugin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be uncommented? if so, we can remove the <!-- Uncomment to add hydrodynamics --> line above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏛️ ionic Gazebo Ionic
Projects
Status: Inbox
Development

Successfully merging this pull request may close these issues.

Update LRAUV model upstream to use new hydrodynamic parameters.
3 participants