Add RigidBody::CalcCenterOfMassTranslationalAccelerationInWorld().

[mitiguy]

Add RigidBody::CalcCenterOfMassTranslationalAccelerationInWorld() towards adding the missing method Multibody::CalcCenterOfMassTranslationalAcceleration() to resolve issue #14269. To see how this PR fits into bigger plan, see PR #21519.

Note: The related velocity function RigidBody::CalcCenterOfMassTranslationalVelocityInWorld() does not have a python binding. For consistency, it seems reasonable that neither should have a Python binding or both should. If it makes sense for these to have Python bindings (and release notes), we'll proceed in a subsequent PR.

---

This change is 

[mitiguy]

Feature review +@joemasterjohn (if you have time).

Reviewable status: LGTM missing from assignee joemasterjohn, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @mitiguy)

[joemasterjohn]

Feature pass done, PTAL.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee joemasterjohn, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @mitiguy)

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 43 at r1 (raw file):

// Note: The applied forces (such as gravity) on this system are irrelevant as
// this text fixture is currently only used to test kinematics for an
// instantaneous given state.  The mass/inertia properties of this plant are

It seems like this fixture (and the test name) is maybe outgrowing it's original purpose since we're not just doing kinematics testing here. We should update this comment at least, and maybe consider a rename for the test.

Code quote:

// Note: The applied forces (such as gravity) on this system are irrelevant as
// this text fixture is currently only used to test kinematics for an
// instantaneous given state.  The mass/inertia properties of this plant are

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 145 at r1 (raw file):

  // Calculate and return [ẇAz, ẇBz], the time derivative of [wAz, wBz].
  Vector2<double> CalcVdotForNoAppliedForces() {
    // The calculation below was generated by MotionGenesis.

I think having the analytic dynamics coming from the symbolic generation is very cool, but I'm wondering if it's the best choice for this test fixture.

1. There is an immediate maintainability issue because most developers will not have access to MotionGenesis, making it difficult to regenerate these if the setup of the fixture changes. And even if the symbolic expressions for the dynamics were deemed necessary, they could be easily regenerated with a MultibodyPlant<symbolic::Expression> version of this system (with gravity turned off).

2. We could just turn gravity off in plant_ and just eval the generalized_acceleration output port of the plant to get Vdot, also eliminating the maintenance requirement on this calculation.

Code quote:

    // The calculation below was generated by MotionGenesis.

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 466 at r1 (raw file):

  // Calculate Bcm's spatial acceleration in world W, expressed in world W.
  const Vector3<double> a_WBcm_W =
      bodyB_->CalcCenterOfMassTranslationalAccelerationInWorld(*context_);

Since we are only considering CoM's that are coincident with body origins, we aren't really getting full coverage in testing CalcCenterOfMassTranslationalAccelerationInWorld().

This test wouldn't detect a sign error or expressed-in error in computing p_BoBcm_W for instance. You can verify this by setting

p_BoBcm_W = p_BoBcm_B

in RigidBody::CalcCenterOfMassTranslationalAccelerationInWorld() and this test will still pass. Or even a typo like:

  const Vector3<T>& w_WB_W = V_WBo_W.translational();

will still pass this test.

I think we need to make body origins and CoM points non-coincident to really give this test some legs.

Code quote:

  // Point Bcm is link B's center of mass, colocated with Bo (B's centroid).
  // Calculate Bcm's spatial acceleration in world W, expressed in world W.
  const Vector3<double> a_WBcm_W =
      bodyB_->CalcCenterOfMassTranslationalAccelerationInWorld(*context_);

[mitiguy]

Thanks Joe. Changes made. PTAL.

Reviewable status: 3 unresolved discussions, LGTM missing from assignee joemasterjohn, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @mitiguy)

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 43 at r1 (raw file):

Previously, joemasterjohn (Joe Masterjohn) wrote…

It seems like this fixture (and the test name) is maybe outgrowing it's original purpose since we're not just doing kinematics testing here. We should update this comment at least, and maybe consider a rename for the test.

Done. Comment updated. Perhaps sufficient?

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 145 at r1 (raw file):

Previously, joemasterjohn (Joe Masterjohn) wrote…

I think having the analytic dynamics coming from the symbolic generation is very cool, but I'm wondering if it's the best choice for this test fixture.

1. There is an immediate maintainability issue because most developers will not have access to MotionGenesis, making it difficult to regenerate these if the setup of the fixture changes. And even if the symbolic expressions for the dynamics were deemed necessary, they could be easily regenerated with a MultibodyPlant<symbolic::Expression> version of this system (with gravity turned off).

2. We could just turn gravity off in plant_ and just eval the generalized_acceleration output port of the plant to get Vdot, also eliminating the maintenance requirement on this calculation.

Done. I simplified the problem so the analysis could be done by-hand and also verified using your suggestion in #2. Let me know if you prefer to remove the by-hand analysis.

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 466 at r1 (raw file):

Previously, joemasterjohn (Joe Masterjohn) wrote…

Since we are only considering CoM's that are coincident with body origins, we aren't really getting full coverage in testing CalcCenterOfMassTranslationalAccelerationInWorld().

This test wouldn't detect a sign error or expressed-in error in computing p_BoBcm_W for instance. You can verify this by setting

p_BoBcm_W = p_BoBcm_B

in RigidBody::CalcCenterOfMassTranslationalAccelerationInWorld() and this test will still pass. Or even a typo like:

  const Vector3<T>& w_WB_W = V_WBo_W.translational();

will still pass this test.

I think we need to make body origins and CoM points non-coincident to really give this test some legs.

Done. I moved the center of mass to the distal end of each link. Let me know what you think.

[mitiguy]

@joemasterjohn PR #21599 seems ready for your review.

Reviewable status: 3 unresolved discussions, LGTM missing from assignee joemasterjohn, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @mitiguy)

[joemasterjohn]

feature, thanks for the changes Paul! +@rpoyner-tri for platform review, please.

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @mitiguy)

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 43 at r1 (raw file):

Previously, mitiguy (Mitiguy) wrote…

Done. Comment updated. Perhaps sufficient?

Looks good!

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 145 at r1 (raw file):

Previously, mitiguy (Mitiguy) wrote…

Done. I simplified the problem so the analysis could be done by-hand and also verified using your suggestion in #2. Let me know if you prefer to remove the by-hand analysis.

OK thanks, I still like having the by-hand math there as a sanity check.

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 466 at r1 (raw file):

Previously, mitiguy (Mitiguy) wrote…

Done. I moved the center of mass to the distal end of each link. Let me know what you think.

This is great, thanks!

[rpoyner-tri]

typo

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @mitiguy)

---

multibody/plant/test/multibody_plant_kinematics_test.cc line 473 at r2 (raw file):

  // Verify results from CalcCenterOfMassTranslationalAccelerationInWorld()
  // match test_utilities::CalcSpatialAccelerationViaAutomaticDifferentiation().
  const Vector3<double> p_AoAcm_A = p_AoAf_A_;  // Af is colocated with Acm.

typo

Suggestion:

collocated

[mitiguy]

+(status: squashing now)

Reviewable status: complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),joemasterjohn

[jwnimmer-tri]

The first line (subject line) of a commit message should be less than 70 characters, and should not mention the issue number.