Enhance documentation and tests for center of mass functions; clarify relationship to forward dynamics #21703
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mitiguy
added
priority: medium
status: single reviewer ok
mitiguy
changed the title
mitiguy
commented
Jul 10, 2024
There was a problem hiding this comment.
For both reviews
Feature review +@rpoyner-tri
Platform review +@rpoyner-tri
Reviewable status: LGTM missing from assignee rpoyner-tri(platform)
rpoyner-tri
reviewed
Jul 11, 2024
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! all discussions resolved, LGTM from assignee rpoyner-tri(platform)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Work to enhance documentation and tests after PR #21599 recently merged.
Note: PR #21599 is work towards issue #14269.
This PR has the following.
Better comments before each TEST_F function in multibody_plant_kinematics_test.cc
Remove antiquated TODO(Mitiguy) commented-out block of code.
Fix varios typos.
Improve exception messages for the center of mass functions MultibodyPlant::CalcJacobianCenterOfMassTranslationalVelocity() and
MultibodyPlant::CalcBiasCenterOfMassTranslationalAcceleration() so they match similar exceptions messages in other center of mass functions. Adds tests for those two exceptions messages (these tests were missing).
Most importantly, added the following comment in rigidy_body.h to
RigidBody:: CalcCenterOfMassTranslationalAccelerationInWorld()
/// @note When cached values are out of sync with the state stored in context,
/// this method performs an expensive forward dynamics computation, whereas
/// once evaluated, successive calls to this method are inexpensive.
It is important to realize that calling MultibodyPlant::CalcSpatialAccelerationsFromVdot() does affect the results of
RigidBody:: CalcCenterOfMassTranslationalAccelerationInWorld(),
as that calculation is always associated with a forward dynamics analysis.
This change is