Implementation of floating-base task space control

nathantpickle-cfdrc commented 4 months ago

Fixes issue #3636

Brief summary of changes

New classes for task space control have been added in OpenSim/Tools. Currently the files are all prefixed with "TaskSpace", but could be renamed and/or moved as needed. Examples are located in OpenSim/Examples/TaskSpace.

A manuscript preprint with the associated mathematical derivation can be found here:

https://www.biorxiv.org/content/10.1101/2024.02.13.580044v1

Testing I've completed

All examples have been tested and are working correctly. All new functionality is located in new classes and does not affect existing code/tests.

Looking for feedback on...

Integration - should the new task classes be integrated with existing tracking tasks (IK, CMC)?
Organization - does the structure of the new code make sense? Should anything be moved/renamed?
Clarity - is all documentation clear?
Style - Are the naming/comment conventions consistent with OpenSim guidelines?

CHANGELOG.md (choose one)

will update once changes are finalized

This change is

aymanhab commented 4 months ago

Build failures are due to the latest PR to use sockets instead of actuator names https://github.com/opensim-org/opensim-core/pull/3683 Sorry for the late change, but please update the code to match the latest conventions/API . Thank you @nathantpickle-cfdrc

nathantpickle-cfdrc commented 4 months ago

Build failures are due to the latest PR to use sockets instead of actuator names #3683 Sorry for the late change, but please update the code to match the latest conventions/API . Thank you @nathantpickle-cfdrc

Thanks @aymanhab, I'll take a look at it.

nickbianco commented 4 months ago

A few more thoughts:

There currently are no tests accompanying these new classes. Examples are good, but a proper test suite with unit tests should be included cover common interface usage and important calculations. Look to CONTRIBUTING.md and the Moco test folder for guidance here. New tests should use the Catch2 testing framework.
I'm wondering if it makes sense to break off the examples into a separate PR, given the volume of new files here. It would speed up the review cycles for the primary new classes and likely make reviewing the examples easier in the follow-up PR.

nathantpickle-cfdrc commented 4 months ago

Thanks @nickbianco and @adamkewley for the initial review! I'll get started on the suggested changes.

@nickbianco I'm on board with your suggestion to separate out the examples into a separate PR. I can remove the examples and add tests that will exercise the new classes.

opensim-org / opensim-core