Remove speed computation from force computation for ScalarActuator

[pepbos]

This PR removes speed computation from the force computation for children of ScalarActuator.

The children of ScalarActuator implemented computeForce by also calculating the speed, and caching this using ScalarActuator::setSpeed. Topologically this seems counter intuitive: You simply do not need speed to compute force. This can have large implications on performance, but that will be a different PR. First step is to make sure that computeForce only computes what it needs to.

To this end, this PR changes the following:

• Changed ScalarActuator::getSpeed to a pure virtual function, since this is the required interface for ScalarActuator to compute power.
• Removed ScalarActuator::setSpeed because it was not used by ScalarActuator, so I assumed it not to be an interface requirement.
• Removed speed cache variable from ScalarActuator because not all Actuators were actually caching the speed (e.g. Millard2012EquilibriumMuscle), so it seems that whether or not to cache, is up to the children.
• For the direct children of ScalarActuator, i.e. TorqueActuator, PointActuator, PointToPointActuator, PistonActuator and PathActuator moved the speed computation out of computeForce and into the getSpeed impl.
• Added caching of speed variable for the above children where needed.

Testing I've completed

• Unit tests passing.

Looking for feedback on

Any feedback on the above considerations is welcome! Also, the naming convention of get in getSpeed: Normally a getter will not imply calculations being done, but I see in many parts of the code that a getter will check if a cached variable is valid, and do some calculations if it was not. I tried to follow this structure. @nickbianco : have a look at MocoStudyFactory, I think that it never computed speed in the first place?

CHANGELOG.md

• updated.

---

This change is 

[pepbos]

Ty for your feedback @nickbianco! I'll get to implementing those.

Indeed some actuators do need to compute speed when computing the force, just as you say. It's just that not all actuators need to. Also, these are derived classes... Topologically it doesnt really seem to make sense to require speed computation, during a force computation.

Perhaps the ScalarActuator::setSpeed function kind of implied that you had to set the speed before the reporter comes along and requests the speed output. If you currently remove speed computation from the force computation you get an exception when the reporter comes along. I'm just guessing here.

But clearly the speed computation is not a requirement for computing the force, because you can split them, as shown.

[nickbianco]

@pepbos I agree that actuators shouldn't require speed computations, I was just speculating on why PathActuator had them within computeForce at all. But after taking a second look, it seems like it was only for power calculations (as the comment suggests), and making getSpeed() pure virtual now makes sure that speed is available when needed. In short, feel free to ignore those comments and sorry for the confusion!

[pepbos]

Hi @nickbianco , about the get set calc convention, and looking at an example: https://github.com/opensim-org/opensim-core/blob/4a3ae55c3580d9cadd3988b2e3ee3e2c0bcfef3b/OpenSim/Simulation/Model/Muscle.cpp#L547 It looks like we want to cache a variable when it is requested. When adding a set method, this method will still be called inside the get method (instead of calc), because get is what others use as interface...

I think the current caching behavior messes a bit with the get set and calc convention...

[pepbos]

ty @tkuchida for reviewing, I changed return values to NaN as you suggested, makes more sense