cellml / libcellml

Repository for libCellML development.
https://libcellml.org
Apache License 2.0
17 stars 21 forks source link

Flattening a model seems to change the model #1174

Closed nickerso closed 1 year ago

nickerso commented 1 year ago

If I have things right, flattening a model should not in any way change the model. If I have this wrong, stop reading here :)

It seems that using the bug.py in the attached zip file I'm seeing the original model being changed following the call to flatten it. Found this when I was accidentally validating the wrong model object and first thought this was a bug in the flattening.

bug.zip

This is using a windows build of the current main HEAD code (if I have things right..)

agarny commented 1 year ago

To flatten a model should clearly not change the model.

I have just tried your bug.py script using the main branch on macOS and I got the following output:

The method "parse_model" found 1 issues:
    - (Message) Given model is a CellML 1.1 model, the parser will try to represent this model in CellML 2.0.
The method "validate_model" found 0 issues:
The method "resolve_imports" found 1 issues:
    - (Message) Given model is a CellML 1.1 model, the parser will try to represent this model in CellML 2.0.
The method "validate_resolved_model" found 0 issues:
The method "validate_flattened_model" found 0 issues:
The method "validate_original_model" found 1 issues:
    - (Error) Imported component 'child_main' is not valid because:
  -> Component 'child_main' importing 'main' from 'child.cellml' has an error:
   - Variable 'TAveabsE' in component 'main' has a units reference 'perS' which is neither standard nor defined in the parent model.
swig/python detected a memory leak of type 'std::shared_ptr< libcellml::Parser > *', no destructor found.
swig/python detected a memory leak of type 'std::shared_ptr< libcellml::Model > *', no destructor found.
swig/python detected a memory leak of type 'std::shared_ptr< libcellml::Validator > *', no destructor found.
swig/python detected a memory leak of type 'std::shared_ptr< libcellml::Importer > *', no destructor found.
swig/python detected a memory leak of type 'std::shared_ptr< libcellml::Model > *', no destructor found.

Is that what you are getting? If so, then yeah, the original model gets changed and this is bad. :/

hsorby commented 1 year ago

Technically it is not the original model that is getting changed, it is the imported child model. The child's units are getting removed resulting in the validation error. But otherwise I agree, it is bad.