Skip to content

Fix Buoyancy state contamination on world reset#3520

Open
XINJIANGMO wants to merge 10 commits intogazebosim:mainfrom
XINJIANGMO:fix_buoyancy_reset
Open

Fix Buoyancy state contamination on world reset#3520
XINJIANGMO wants to merge 10 commits intogazebosim:mainfrom
XINJIANGMO:fix_buoyancy_reset

Conversation

@XINJIANGMO
Copy link
Copy Markdown
Contributor

@XINJIANGMO XINJIANGMO commented May 7, 2026

🦟 Bug fix

Depends on #3503

Summary

During the reset audit, the Buoyancy plugin failed the tests. After an environment reset, the model didn’t consistently return to its early-episode trajectory.

Test logic

  1. Recorded an early trajectory window of a model in graded_buoyancy.sdf before resetting.
  2. Let the simulation run for a while, ensuring the system moves explicitly away from its initial state.
  3. Executed a world reset.
  4. Recorded the post-reset trajectory for the exact same early-episode time window.
  5. Compared both trajectories to ensure they match identically, confirming whether the reset successfully brought the system back to a clean initial state.

Root cause

The issue stems from how Buoyancy tracks the running state of links. It originally relied on the EachNew entity scan to fetch and cache components like Volume and CenterOfVolume. When the world resets, the entities are restored but they aren't strictly considered "new" again by the ECM. This led to broken buoyancy states where required components couldn't be reliably rebuilt on reset.

Fix logic

  1. Add ISystemReset for the Buoyancy
  2. Clear the cached states: centerOfVolumes,volumes and buoyancyForces
  3. Introduce a rescanEntities flag. Once a reset occurs, this flag triggers a fresh, full entity component scan in the very next PreUpdate step, gracefully recovering the required runtime components.

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • 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)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Generated-by: Remove this if GenAI was not used.

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

Backports: If this is a backport, please use Rebase and Merge instead.

XINJIANGMO added 9 commits May 5, 2026 18:10
Signed-off-by: momo <2438833481@qq.com>
Signed-off-by: momo <2438833481@qq.com>
Signed-off-by: momo <2438833481@qq.com>
Signed-off-by: momo <2438833481@qq.com>
Signed-off-by: momo <2438833481@qq.com>
Signed-off-by: momo <2438833481@qq.com>
Signed-off-by: momo <2438833481@qq.com>
Signed-off-by: momo <2438833481@qq.com>
@XINJIANGMO XINJIANGMO marked this pull request as ready for review May 8, 2026 03:50
@XINJIANGMO XINJIANGMO requested a review from arjo129 as a code owner May 8, 2026 03:50
Copilot AI review requested due to automatic review settings May 8, 2026 03:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a world-reset bug in the Buoyancy system where cached per-link state (and missing runtime components) could survive across resets, causing post-reset motion to diverge from the initial “early-episode” trajectory.

Changes:

  • Implement ISystemReset in Buoyancy to clear cached state and trigger a one-time full entity rescan after reset.
  • Update entity scanning logic to optionally use a full Each pass (instead of EachNew) immediately after reset.
  • Add an integration test that records model pose trajectories before/after reset in graded_buoyancy.sdf and asserts they match within tolerance.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
test/integration/buoyancy.cc Adds a reset regression test validating trajectory equivalence pre/post reset.
src/systems/buoyancy/Buoyancy.hh Extends the system interfaces to include ISystemReset and declares Reset.
src/systems/buoyancy/Buoyancy.cc Clears cached data on reset and performs a one-time full link scan to rebuild runtime components.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

3 participants