Muscle FiberVelocityInfo and MuscleDynamicsInfo refactor #14

pepbos · 2023-12-08T08:07:28Z

This PR combines Muscle::FiberVelocityInfo and Muscle::MuscleDynamicsInfo into a single cached variable, and removes member fields that are cheap to compute.

Considering that fetching memory is slower than multiplying a few numbers, the following fields are cheap to compute, and are removed:

FiberVelocityInfo::fiberVelocityAlongTendon,
FiberVelocityInfo::normFiberVelocity,
FiberVelocityInfo::pennationAngularVelocity,
FiberVelocityInfo::normTendonVelocity,
MuscleDynamicsInfo::fiberForceAlongTendon,
MuscleDynamicsInfo::normFiberForce,
MuscleDynamicsInfo::normTendonForce,
MuscleDynamicsInfo::fiberStiffnessAlongTendon,
MuscleDynamicsInfo::muscleStiffness,
MuscleDynamicsInfo::tendonPower,
MuscleDynamicsInfo::musclePower,

This leaves two fields in FiberVelocityInfo.
When computing the fiber velocity, with the exeption of Millard2012AccelerationMuscle, all muscles need to compute the forces as well. The forces can thus be seen as a byproduct of computing the velocities,
Furthermore, the FiberVelocityInfo and MuscleDynamicsInfo cached variables are invalidated at the same stage (Stage::Velocity).
Finally, it turns out that distributing the Muscle information over 3 cached variables is slower than over 2.

Since computing the forces is actually part of computing the muscle velocities, I argue for combining MuscleDynamicsInfo with FiberVelocityInfo.

Performance

Measuring the wall clock time for several simulations, shows around ~10% performance benefit:

	main	this PR
CMC Running Model	26.0 [secs]	21.4 [secs] (-22%)
Rajagopal forward integration	6.5 [secs]	5.9 [secs] (-10%)

Brief summary of changes

Moved all fields from MuscleDynamicsInfo to FiberVelocityInfo,
Removed Muscle::calcMuscleDynamicsInfo(), Muscle::getMuscleDynamicsInfo and the cached variable.
Added cached variable to Millard2012AccelerationMuscle for caching the muscle-specific curve values.
Added cached variable FiberVelocityInfoCache containing the MuscleLengthInfo and FiberVelocityInfo in a single struct
Moved fiberForceLengthMultiplier and fiberActiveForceLengthMultiplier from MuscleLengthInfo to FiberVelocityInfo (since they are used to compute forces/velocities and not lengths).
Added calc methods on FiberVelocityInfoCache variable for the removed fields mentioned above.

Some notes:

I think a more fitting name for the combined cached variable would be MuscleForceInfo, but adding that to this PR will create a very large diff, so that would be a follow up PR.
TODO: Thelen output changed, still debugging.
TODO: DeGrooteMuscle unit tests are failing on small numerical differences (<1e-13).

Testing I've completed

Rajagopal simulations showed no change in output.
Thelen simulation showing no change in output.
Unit tests passing

Looking for feedback on

@aseth1 would be great if you have time to take a look at Muscle.h. I moved the curve computations from MuscleLengthInfo to FiberVelocityInfo because they appeared a byproduct of computing the fiber force. But I might have misunderstood the invalidating behavior of CMC.
@adamkewley I left the calc methods in Muscle.h in the header e.g. Muscle.h::685