Skip to content

Conversation

@nickbianco
Copy link
Member

@nickbianco nickbianco commented Apr 7, 2025

Fixes issue #4042

Brief summary of changes

This change exposes the "dissipated energy" state variable allocated by the SimTK::Force::LinearBushing that is internal to BushingForce. This fixes the bug in #4042, which occurred because the size of the auxiliary state vector reserved by Moco did not match SimTK::State::getZ(). This happened because Moco reserves state vectors based on the number of state variables reported by each Component, but BushingForce did not report the "dissipated energy" state as part of the system, even though a slot for it is reserved in the SimTK::State.

For convenience, I added the Output "statebounds_dissipated_energy", which Moco will use to automatically set the bounds on the energy dissipation state variable (i.e., so users do not have to call setStateInfo).

Testing I've completed

Added tests to testMocoInterface.cpp and testForces.cpp.

Looking for feedback on...

CHANGELOG.md (choose one)

  • updated.

This change is Reviewable

@nickbianco nickbianco requested a review from carmichaelong April 7, 2025 18:20
@nickbianco
Copy link
Member Author

FYI @nicos1993 and @stingjp.

@nickbianco nickbianco requested review from aymanhab and removed request for carmichaelong April 7, 2025 22:05
Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 4 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nickbianco)


CHANGELOG.md line 93 at r2 (raw file):

- Fixed bugs in `PolynomialPathFitter` when too few coordinate samples were provided. (#4039)
- Exposed the "dissipated energy" state variable allocated by the `SimTK::Force::LinearBushing` that is internal to `BushingForce`. 
  This changed fixed a bug in Moco, where adding a `BushingForce` led to a segfault due to a mismatch between the size of the 

typo (changed -> change?)

Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nickbianco)

@nickbianco
Copy link
Member Author

Thanks @aymanhab!

@nickbianco nickbianco merged commit d409fd8 into main Apr 7, 2025
1 check was pending
@nickbianco nickbianco deleted the moco_bushing_force_bug_fix branch April 7, 2025 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants