-
Notifications
You must be signed in to change notification settings - Fork 291
Fix mimic/coupler constraints to mirror Gazebo mimic repro #2247
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
Conversation
…ead of local followers - Updated configureMimicMotors in both example and test to get reference joints from middle pendulum (pendulum_with_base) instead of same skeleton - Modified MimicPairView to track middleReference from the middle pendulum - Updated GUI renderMimicTable to show error against middle pendulum reference - Made GUI widget resizable with initial size 800x600 - Added ImGuiTableFlags_Resizable for resizable table columns - Per-step mimic tracking ensured via MimicMotorConstraint's ERP/CFM parameters This fixes gazebosim/gz-physics#432 issue where followers now properly track the correct reference joints over extended simulations.
# Conflicts: # dart/constraint/CouplerConstraint.hpp # dart/constraint/MimicMotorConstraint.hpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
scpeters
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testing in progress
f0389cc to
15b47b3
Compare
Thank you for the comments! Fixed the build errors and cleaned up the .sdf file as well |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2247 +/- ##
=======================================
Coverage 60.82% 60.83%
=======================================
Files 353 353
Lines 31880 31911 +31
Branches 4128 4139 +11
=======================================
+ Hits 19392 19412 +20
- Misses 12488 12499 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
scpeters
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this is more stable, but I still see energy increasing enough that the pendulum swings cause displacement of the bases
here is a 10x screen recording. the pendulums start parallel to the ground (initial angle 90 degrees from vertical) with no kinetic energy, and this is a passive system, so none of the pendulums should swing above the horizontal, but I see both pendulums swing above horizontal at some points, though the slow_follows_fast model on the left has the most dramatic energy gain
dartsim.mimic.pendulums.energy.gain.Screen.Recording.2025-11-23.at.8.23.48.PM.mp4
I think this is fine to merge; it is better, though not yet best as there's still room for improvement in the numerical stability when using the coupler constraint
|
Thanks for running the long test! I hadn’t exercised it that far yet, so I didn’t see this. I’ll dig in and address it in a follow-up. |
This reverts commit 3f2a829.
Summary
Testing
pixi run test-allpixi run ex mimic_pendulumsScreencast.from.2025-11-23.11-02-56.mp4
Before creating a pull request
pixi run test-allto lint, build, and test your changes