search

SciML / CellMLToolkit.jl

CellMLToolkit.jl is a Julia library that connects CellML models to the Scientific Julia ecosystem.
https://docs.sciml.ai/CellMLToolkit/stable/
Other
62 stars 15 forks source link

Read .cellml file directly into ODESystem in components form #13

Closed anandijain closed 3 years ago

anandijain commented 3 years ago

There is little reason to have a CellModel type. Especially since it does not subtype MTK.AbstractSystem making usage clunky.

It would be much better if we could do something like read(ODESystem, "mymodel.cellml").

The result should match the components form of the acausal modeling tutorial

Thoughts?

shahriariravanian commented 3 years ago

We need a placeholder for all the various data in a CellML XML file (variables, units, ODE, and algebraic equations) during parsing and code generation. We can add a read function and not expose the user to CellModel.

generate_functions is a helper function, mainly for debugging purposes. Many CellML files have bugs and won't compile completely. It can be difficult to find the bugs based on the MTK error messages and variable names, especially as some CellML models are rather large and convoluted (40-50 state variables with hundreds of equations). I think it is a good idea to have the possibility of generating a human-readable file that can be tweaked and debugged manually. Of course, most users should just plug the CellML file into a function like read and not worried about all the internal stuff.

anandijain commented 3 years ago

Yeah I opened a draft PR for this. It seems like there is a lot of duplicated code in here that is only here because CellModel doesn't subtype System. list_states list_params flat_equations seem to be almost exactly MTKs' accessor functions.

To me, it seems like the only CellModel field that is different from MTK is units, as I believe that we don't need to worry about whether it is an ODE or algebraic equation. I have the impression that MTK should handle it now.

Would you be willing to help me sort out some of my CI errors and how to make read or in this case load_cellml "just work"?

shahriariravanian commented 3 years ago

I agree. We can ignore units for now. Eventually, it needs to be integrated with Unitful.jl or something similar.

I suggest a two-step process. We can add a read function on top of the current CellML system to hide the details and stabilize the API. Then, we can port the internals to MTK functions.

anandijain commented 3 years ago

Great, I think this is a good plan. I'd love some review on my PR which is a first shot at step 1.

I implemented the read in load_cellml, but was having trouble with my env and CI, so I wasn't able to get tests to pass. Help on this would be appreciated.