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

Cleanup: Muscle state estimate return type in Millard2012QuilibriumMuscle #3500

Conversation

pepbos
Copy link
Contributor

@pepbos pepbos commented Jun 27, 2023

This PR adds a struct for capturing the output of the internal function Millard2012QuilibriumMuscle::estimateMuscleFiberState.

Previously the output was stored in a std::pair, with the second field a std::map. This made it less clear what exactly the returned values are. Since the returned information was always the same, this PR adds a struct to hold it.

Brief summary of changes

Adds MuscleStateEstimate to capture the output of Millard2012QuilibriumMuscle::estimateMuscleFiberState.

Testing I've completed

Visual verification that Rajagopal model simulation gives same result before and after change.
Passed the unit tests.

CHANGELOG.md

  • no need to update because minor internal refactor.

This change is Reviewable

Replaces the std::map<...>, which was the return type of
`Millard2012EquilibriumMuscle::estimateMuscleFiberState`
MuscleStateEstimatorResult to MuscleStateEstimate
and member fields in OpenSim style.
@pepbos pepbos marked this pull request as ready for review June 28, 2023 15:30
@pepbos
Copy link
Contributor Author

pepbos commented Jun 29, 2023

@adamkewley thanks for your comments! @tkuchida , @mjhmilla , @nickbianco, care to take a look?

@mjhmilla
Copy link
Contributor

I'll take a look once my conference presentation drafts are ready. Hopefully sometime next week.

@pepbos pepbos requested a review from adamkewley July 18, 2023 08:26
@adamkewley
Copy link
Contributor

LGTM 👍

Copy link
Member

@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.

LGTM!

@adamkewley adamkewley merged commit e6a1b25 into opensim-org:main Jul 19, 2023
6 checks passed
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.

5 participants