Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Moco 3D tracking example with foot-ground contact #4008

Merged
merged 42 commits into from
Feb 20, 2025

Conversation

nickbianco
Copy link
Member

@nickbianco nickbianco commented Jan 31, 2025

Fixes issue #3960

Brief summary of changes

This change adds example3DWalking.cpp, an example that demonstrates how to create a simulation of walking tracking both kinematics and ground reaction forces using a contact model. This example lives inside the example3DWalking folder and relies on the existing reference data there, some of which was modified to accommodate the new example while still working for exampleMocoInverse and exampleMocoTrack. New reference files were added to allow adding contact forces to the model and for loading in marker weights based on an IKTaskSet.

The following minor additions are included in this PR which are incorporated in the new/updated examples:

  • Added ModelFactory::createResidualActuators and ModOpAddResiduals to make it easy to add a set of residual actuators to a model.
  • Updated ModelFactory::createReserveActuators and ModOpAddReserves with changes that make them compatible with the new residual utility methods.
  • Added convenience methods to MocoTrack to allow setting marker weights from a Set<MarkerWeight> or IKTaskSet.
  • Added a const-qualifier to IKTaskSet::createMarkerWeightSet.
  • Minor improvements and changes to exampleMocoInverse and exampleMocoTrack based on the updated reference data and the new utility methods.

Testing I've completed

WIP: testing all updated examples to make sure results are consistent across C++, Python, and Matlab.

Looking for feedback on...

I tried to check carefully that all updated examples are consistent with each other, but additional spot checking during review would be helpful.

CHANGELOG.md (choose one)

  • updated.

This change is Reviewable

@nickbianco nickbianco marked this pull request as draft January 31, 2025 21:50
@nickbianco nickbianco marked this pull request as ready for review February 6, 2025 01:32
@nickbianco nickbianco changed the title [WIP] Add Moco 3D tracking example with foot-ground contact Add Moco 3D tracking example with foot-ground contact Feb 11, 2025
Copy link
Member

@carmichaelong carmichaelong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome to have an example like this. looks good overall, just some minor comments

Reviewed 23 of 23 files at r1, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @aymanhab)


Bindings/Python/examples/Moco/example3DWalking/exampleMocoTrack.py line 219 at r1 (raw file):

    problem = study.updProblem()

    # Set the control effort weights.

i wonder if this is a good place to be a little more explicit in the comments here to emphasize how this sets a weight of 0.1 for all controls except for the pelvis wiht a weight of 10 using a wildcard pattern (something of that nature)


OpenSim/Examples/Moco/example3DWalking/subject_walk_scaled_ContactForceSet.xml line 77 at r1 (raw file):

                <socket_half_space>/contactgeometryset/floor</socket_half_space>
                <!--The stiffness constant (i.e., plain strain modulus), default is 1 (N/m^2)-->
                <stiffness>306777.59999999998</stiffness>

nit: this and the left side have slightly different stiffness than the other contact spheres (~1.6 larger)


CHANGELOG.md line 76 at r1 (raw file):

- Remove unused, legacy `<defaults>` blocks in `.osim` and `.xml` files (#3986).
- Added `example3DWalking`, which demonstrates how to create a tracking simulation with foot-ground contact in Moco. (#4008)
- Added `ModelFactory::createResidualActuators` and `ModOpAddResiduals` to make it easy to add a set of residual actuators to a model. (#4008)

could also mention the change to the similar function for reserves so that folks know they can now set reserves and residuals separately along with this new function


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 43 at r1 (raw file):

    % Set minimum muscle controls and activations to 0 (default is 0.01).
    actuators = model.updActuators();

can you use updMuscles() here?


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 178 at r1 (raw file):

    % Construct the MocoTrack problem.
    track = MocoTrack();
    track.setName('track_walking');

can use studyName here


Bindings/Python/examples/Moco/example3DWalking/example3DWalking.py line 86 at r1 (raw file):

    # Construct the MocoTrack problem.
    track = osim.MocoTrack()
    track.setName('track_walking')

can use study_name here


Bindings/Python/examples/Moco/example3DWalking/example3DWalking.py line 237 at r1 (raw file):

# Set minimum muscle controls and activations to 0 (default is 0.01).
actuators = model.updActuators()

is it possible to use updMuscles() instead and then not have to use the if?


OpenSim/Examples/Moco/example3DWalking/subject_walk_scaled.osim line 14393 at r1 (raw file):

					<socket_parent_frame>/bodyset/toes_l</socket_parent_frame>
					<!--The fixed location of the station expressed in its parent frame.-->
					<location>0.0877793 0.0428623 0.0226433</location>

this isn't symmetric with marker R.Toe. intentional?


OpenSim/Examples/Moco/example3DWalking/subject_walk_scaled.osim line 14409 at r1 (raw file):

					<socket_parent_frame>/bodyset/toes_l</socket_parent_frame>
					<!--The fixed location of the station expressed in its parent frame.-->
					<location>0.00474658 0.0240174 -0.0659469</location>

this isn't symmetric with marker R.MT5. intentional?

Copy link
Member Author

@nickbianco nickbianco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed review @carmichaelong! I've addressed all of your comments.

Reviewable status: 15 of 23 files reviewed, 9 unresolved discussions (waiting on @aymanhab and @carmichaelong)


CHANGELOG.md line 76 at r1 (raw file):

Previously, carmichaelong wrote…

could also mention the change to the similar function for reserves so that folks know they can now set reserves and residuals separately along with this new function

Done.


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 43 at r1 (raw file):

Previously, carmichaelong wrote…

can you use updMuscles() here?

Done.


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 178 at r1 (raw file):

Previously, carmichaelong wrote…

can use studyName here

Done.


Bindings/Python/examples/Moco/example3DWalking/example3DWalking.py line 86 at r1 (raw file):

Previously, carmichaelong wrote…

can use study_name here

Done.


Bindings/Python/examples/Moco/example3DWalking/example3DWalking.py line 237 at r1 (raw file):

Previously, carmichaelong wrote…

is it possible to use updMuscles() instead and then not have to use the if?

Done.


Bindings/Python/examples/Moco/example3DWalking/exampleMocoTrack.py line 219 at r1 (raw file):

Previously, carmichaelong wrote…

i wonder if this is a good place to be a little more explicit in the comments here to emphasize how this sets a weight of 0.1 for all controls except for the pelvis wiht a weight of 10 using a wildcard pattern (something of that nature)

Done.


OpenSim/Examples/Moco/example3DWalking/subject_walk_scaled.osim line 14393 at r1 (raw file):

Previously, carmichaelong wrote…

this isn't symmetric with marker R.Toe. intentional?

Done.


OpenSim/Examples/Moco/example3DWalking/subject_walk_scaled.osim line 14409 at r1 (raw file):

Previously, carmichaelong wrote…

this isn't symmetric with marker R.MT5. intentional?

Done.


OpenSim/Examples/Moco/example3DWalking/subject_walk_scaled_ContactForceSet.xml line 77 at r1 (raw file):

Previously, carmichaelong wrote…

nit: this and the left side have slightly different stiffness than the other contact spheres (~1.6 larger)

It's actually a factor of 10 lower than the other spheres. Hmm, I hadn't noticed that before. I borrowed these values from Antoine's simulations. I wonder if this is intentional since these spheres are applied to the low-mass toes. Since the example is working as intended, I'm inclined to leave these values as is.

Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great, I meanly wanted explanations for why selections/Since you need to have cpp, py, matlab versions it may make sense to have a readme file explaining the decisions that all flavors go back to, otherwise LGTM

Reviewed 1 of 23 files at r1, 7 of 8 files at r2, all commit messages.
Reviewable status: 22 of 23 files reviewed, 16 unresolved discussions (waiting on @carmichaelong)


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 54 at r2 (raw file):

    ebcf_toes_l = ExpressionBasedCoordinateForce(...
            'mtp_angle_l', '-25.0*q-2.0*qdot');
    ebcf_toes_l.setName('toe_damping_l');

What's the rationale for this expression?


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 65 at r2 (raw file):

    ca_toes_l.setName('mtp_angle_l_actuator');
    ca_toes_l.setOptimalForce(50);
    ca_toes_l.setMinControl(-1.0);

justification for 50?


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 90 at r2 (raw file):

    end
    model.finalizeConnections();

Do we need this finalizeConnections? if so maybe document the reason


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 105 at r2 (raw file):

    % Solve a torque-driven tracking problem to createa a kinematic 
    % trajectory that is consistent with the ground reaction forces.

create a


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 152 at r2 (raw file):

        contactTrackingWeight = 1e-2;
    end

Some justification of relative weights may help


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 242 at r2 (raw file):

            coordName = string(coordinate.getName());
            if (contains(coordName, 'beta')) 
                continue; 

Rationale for excluding 'beta' coordinates


Bindings/Java/Matlab/examples/Moco/example3DWalking/exampleMocoTrack.m line 61 at r2 (raw file):

modelProcessor = ModelProcessor("subject_walk_scaled.osim");
% Replace the PinJoints representing the model's toes with WeldJoints.
jointsToWeld = StdVectorString();

Why?

Copy link
Member Author

@nickbianco nickbianco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aymanhab! I've addressed all comments.

I'm currently testing locally if increasing the stiffness of the toe body contact forces changes the solutions in any meaningful way. Once that's cleared up, we could merge.

Reviewable status: 22 of 23 files reviewed, 16 unresolved discussions (waiting on @aymanhab and @carmichaelong)


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 54 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

What's the rationale for this expression?

Done.


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 65 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

justification for 50?

Done.


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 90 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Do we need this finalizeConnections? if so maybe document the reason

Done.


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 105 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

create a

Done.


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 152 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Some justification of relative weights may help

Done.


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 242 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Rationale for excluding 'beta' coordinates

Done.


Bindings/Java/Matlab/examples/Moco/example3DWalking/exampleMocoTrack.m line 61 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Why?

Done.

Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 8 files at r2, 6 of 6 files at r3, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @carmichaelong)

Copy link
Member

@carmichaelong carmichaelong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! all my comments are addressed. i'm ok either way with the toe stiffness.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nickbianco)

@nickbianco
Copy link
Member Author

@aymanhab, @carmichaelong, I made a handful a changes to the example based on comments from the demos meeting yesterday. Mind taking a quick final pass on them before we merge? After adding in ExpressionBasedCoordinateForces at the joints, the muscle-driven problem now converges in ~35 minutes. I also added a QoL fix: demote the warning in DeGrooteFregly2016Muscle regarding activation dynamics to debug so it doesn't spam the console output.

Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example looks much cleaner with the forces moved to a separate file that's reused by all flavors (matlab, cpp, python) also great that it now solves faster.
:lgtm:

Reviewed 6 of 6 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @carmichaelong)

@nickbianco
Copy link
Member Author

@carmichaelong, this is ready for a final review.

Copy link
Member

@carmichaelong carmichaelong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed a small typo in a comment that was propagated through the .cpp, .py, and .m files. Feel free to skip CI for these changes and then merge.

Reviewed 4 of 4 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), 3 unresolved discussions (waiting on @nickbianco)


Bindings/Java/Matlab/examples/Moco/example3DWalking/example3DWalking.m line 101 at r5 (raw file):

% Construct a MocoStudy to track joint kinematics and ground reaction forces
% using a torque-driven or muscle-driven model with foot-ground contact
% elements. The objective function weights were chosen such the optimized 

typo: were chosen such that the optimized


OpenSim/Examples/Moco/example3DWalking/example3DWalking.cpp line 38 at r5 (raw file):

/// Construct a MocoStudy to track joint kinematics and ground reaction forces 
/// using a torque-driven or muscle-driven model with foot-ground contact 
/// elements. The objective function weights were chosen such the optimized 

typo: such that


Bindings/Python/examples/Moco/example3DWalking/example3DWalking.py line 32 at r5 (raw file):

# Construct a MocoStudy to track joint kinematics and ground reaction forces
# using a torque-driven or muscle-driven model with foot-ground contact
# elements. The objective function weights were chosen such the optimized 

typo: such that

Copy link
Member Author

@nickbianco nickbianco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @carmichaelong (and @aymanhab)! Merging.

Reviewable status: 22 of 25 files reviewed, all discussions resolved (waiting on @carmichaelong)

@nickbianco nickbianco merged commit 3aaae00 into main Feb 20, 2025
1 check was pending
@nickbianco nickbianco deleted the example_3d_walking branch February 20, 2025 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants