cellml / libcellml

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

Imports aren't correctly resolved in simple cellml 1.1 model #1207

Closed FinbarArgus closed 9 months ago

FinbarArgus commented 9 months ago

I have a model that I have flattened that isn't being read as an ODE by the analyser, even though the original model runs in OpenCOR. it is Cellml 1.1

The model and the flat model are (converted from .cellml to .txt)

delay_test.txt delay_test_flat.txt delay_test_modules.txt delay_test_parameters.txt delay_test_units.txt

agarny commented 9 months ago

Here is a ZIP file that contains @FinbarArgus' various CellML files (easier to download and no need to rename anything). Will check things tomorrow.

agarny commented 9 months ago

A lot of issues... with the model itself.

Remember, it's not because it works in OpenCOR (using our legacy CellML API) that it is correct. The CellML API is known to consider some models valid when they should have in fact been considered invalid. So, if a model doesn't work as expected with libCellML, I would output the issues it reports.

By doing what I recommend, I found various issues with interfaces and units in your CellML 1.1 model and, therefore, its flattened version. Attached is the fixed version of your files. Note that the units may not necessarily be what they should be. I merely fixed them to get your model to work.

Bottom line: nothing wrong with libCellML (but certainly with the CellML API!), hence closing.

FinbarArgus commented 9 months ago

Thanks Alan! All fixed.

Strangely, this attached model opens without issue, even though it has the same problems as before. Hence, why I incorrectly assumed it was an import issue. delay_test.zip

Naive question, how do I get the issues? The docs (https://libcellml-tutorials.readthedocs.io/en/latest/common_users/simulation_tool_dev.html) say to use print_issues_to_terminal but I can't find from where to import this function.

nickerso commented 9 months ago

@FinbarArgus - print_issues_to_terminal is a method that you'd need to define in your code, I think somewhere in that example code supporting the tutorials there are some standard methods like that which you could reuse from here: https://github.com/kerimoyle/libcellml-tutorials/tree/develop/tutorials/utilities

agarny commented 9 months ago

Strangely, this attached model opens without issue, even though it has the same problems as before. Hence, why I incorrectly assumed it was an import issue. delay_test.zip

I can confirm that there are a lot issues with units in there (by just looking at the CellML code itself) and, yet, libCellML's analyser is indeed happy with it! This is clearly wrong, hence issue #1208.

agarny commented 9 months ago

FWIW, libCellML's analyser currently allows equations with non-matching units. There is indeed a plan to allow for strict or relaxed units checking in the analyser (see issue #824). (So, closing issue #1208 in favour of issue #824.)

hsorby commented 9 months ago

Could I encourage you @FinbarArgus, to use the documentation available on libcellml.org, specifically: https://libcellml.org/documentation/v0.5.0/tutorials/common_use_cases/simulation_tool_dev.

And I would also encourage you to not use unmaintained and unofficial code but maintained and official code from here: https://github.com/libcellml/tutorials/blob/main/resources/code/utilities/utilities.py, this file is also available from the resource section in https://libcellml.org/documentation/v0.5.0/tutorials/tutorial4/index.