search

opensim-org / opensim-core

SimTK OpenSim C++ libraries and command-line applications, and Java/Python wrapping.
https://opensim.stanford.edu
Apache License 2.0
789 stars 317 forks source link

Bug in loading of Thelen Muscle Parameters #2553

Open MarliesNit opened 5 years ago

MarliesNit commented 5 years ago

We encountered a problem, when loading two different osim-models in the same Matlab session.

Models:

Scenario 1:

Scenario 2:

Affected Versions:

jimmyDunne commented 5 years ago

Hi, Marlies-

Thanks for opening an issue!

I am having trouble reproducing your observed behavior. I have attached a zip that contains two models as you have described— one custom model with altered KShapeActive and Flen properties for the rect_fem_r muscle, and one default model that has KShapeActive and Flen properties commented out for the rect_fem_r_muscle. Below is the included script to perform a test of the property values of the two models. 

clear all; close all; clc
%%
import org.opensim.modeling.*
CustomModelName  = 'gait2392_simbody_customRecFem.osim';
DefaultModelName = 'gait2392_simbody_defaultRecFem.osim';

%% Custom Model 
% Instantiate Model
model1 = Model(CustomModelName);
% Get the rect_fem_r muscle
m = model1.getMuscles().get('rect_fem_r');
% SafeDownCast to Thelen2003Muscle
recFem = Thelen2003Muscle.safeDownCast(m);
% Get the KShape and Flen Properties
% Custom values from osim file are Kshape = 0.7 & Flen = 1.8
Kshape_Custom = recFem.get_KshapeActive();
flen_Custom = recFem.get_Flen();
% Test the Values are correct
if Kshape_Custom ~= 0.7 || flen_Custom ~= 1.8
    error('Custom Values are incorrect')
end

%% Default Model 
% Instantiate Model
model2 = Model(DefaultModelName);
% Get the rect_fem_r muscle
m = model2.getMuscles().get('rect_fem_r');
% SafeDownCast to Thelen2003Muscle
recFem = Thelen2003Muscle.safeDownCast(m);
% Get the KShape and Flen Properties
% Default values that are generated when no values are  
% found in osim file are Kshape = 0.45 & Flen = 1.4
Kshape_Default = recFem.get_KshapeActive();
flen_Default = recFem.get_Flen();
% Test the Values are correct
if Kshape_Default ~= 0.45 || flen_Default ~= 1.4
    error('Custom Values are incorrect')
end

Scenario 2: For model2, I commented out the rect_fem_r KShapeActive and Flen values in the model file (.osim). Once instantiated, model2 shows KShape and Flen values defined by the Thelen2003Muscle() constructor, as expected. The value for KShapeActive is 0.45 and Flen is 1.4.

Scenario 1: I don't see the behavior you described. Model1 shows the (expected) KShape and Flen values defined in the file and model2 shows the (expected) default values.

Please let me know if you something different in the attached test than what you are doing?

-J

test_muscle_properties.zip

aymanhab commented 5 years ago

I think the key is to provide the model files since users can change the default settings for all future instances of a specific type in the osim file itself.

MarliesNit commented 5 years ago

@jimmyDunne, you are right. The problem does not occur for your example.

@aymanhab, yes, that's the key. In our model 1 based on Hamner's model we define a default Thelen muscle in the osim file. However, in my opinion this default parameters should be only valid within this model. Hence, they should not be valid anymore if I load a new osim file. Otherwise, the specific model is never fully defined by the code and the osim file but depends on the model used before.

aymanhab commented 5 years ago

@Marlies93 I agree with your assessment that this is a bug, leaking muscle parameters between models. It doesn't affect most users because the models we distribute do not typically have default objects, there would also be no problem if all models had default objects, it's only when you mix/match models with and without default objects. Will keep this open so it doesn't fall through the cracks, in the meantime please pass this info along to interested parties using these models. Thank you 👍

MarliesNit commented 5 years ago

Thanks for your help!

aymanhab commented 5 years ago

@apoorvar I saw a default muscle in your model distribution, would it be possible to push the changes to individual muscles to remove the default? Other places @jimmyDunne where defaults could be leaking? Thank you.

jimmyDunne commented 5 years ago

Some of the tool setup files have defaults. My guess would be this wouldn't be an issue in CMC ( since CMC doesn't generate a new model), but perhaps there is some case where a tool could generate a new model using these defaults (RRA maybe?)

apoorvar commented 5 years ago

Should be possible. I'd have to check, but I think the default muscle just had the default muscle-model values for any given parameter, so we possibly could just remove the default muscle all together with no effect to the model.

On Tue, Sep 10, 2019 at 9:59 AM jimmyDunne notifications@github.com wrote:

Some of the tool setup files have defaults https://github.com/opensim-org/opensim-models/blob/554c54ef2805fd5ae25b5f9a6ef315754a6a6155/Tutorials/Estimating_Joint_Reaction_Loads/gait2392_SO_weak_Residual_Actuators.xml. My guess would be this wouldn't be an issue in CMC ( since CMC doesn't generate a new model), but perhaps there is some case where a tool could generate a new model using these defaults (RRA maybe?)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opensim-org/opensim-core/issues/2553?email_source=notifications&email_token=ABY5KQP4O4X5LERIOPAYS53QI7G7NA5CNFSM4ITSB7C2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6LZQNA#issuecomment-530028596, or mute the thread https://github.com/notifications/unsubscribe-auth/ABY5KQM2W65CISRRK7JVGFLQI7G7NANCNFSM4ITSB7CQ .

aymanhab commented 5 years ago

Typically these Actuators are fully specified in the Actuators file, it's model components that can lead to this issue. We should discuss solutions and tradeoffs of convenience vs. error-prone: