SciML / ReactionNetworkImporters.jl

Julia Catalyst.jl importers for various reaction network file formats like BioNetGen and stoichiometry matrices
https://docs.sciml.ai/ReactionNetworkImporters/stable/
MIT License
26 stars 8 forks source link

Directly Target ModelingToolkit? #17

Closed ChrisRackauckas closed 3 years ago

ChrisRackauckas commented 4 years ago

The importers in this library don't work too well when used from inside of a function or from an alternative module (distributed) because of the eval going on in there. Now that it's ready, it probably makes sense to target the intermediate language instead of a macro, which will generally have these issues. I wanna start getting some really big problems into MTK so that we can have good test cases for Julia Lab folk to work on things like https://github.com/SciML/ModelingToolkit.jl/issues/366 and https://github.com/JuliaSymbolics/SymbolicUtils.jl/issues/62

isaacsas commented 4 years ago

This will get a rewrite once we are done with the MT and DEBio updates. The eval issue is also one reason I was suggesting we drop custom macro generated types for the reaction_network a while back... I could go right to a ReactionSystem, though in that case I'd prefer if we update the DEBio API to work on ReactionSystems, and just have the reaction_network macro return a ReactionSystem. It'd be nice to have just one API that can handle extending, querying and modifying networks. Either it works on reaction_networks as the container or ReactionSystems...

We should probably get that settled though before @TorkelE finishes the DEBio rewrite. If we go with making everything based on ReactionSystems I'd suggest maybe keeping most of the functionality in DEBio to keep the exports from MT down. Users who want anything beyond basic ReactionSystem functionality could just import DEBio too.

BTW, could the BCR network even be loaded right now, or would it hit this GG issue: https://github.com/thautwarm/GeneralizedGenerated.jl/issues/45

ChrisRackauckas commented 4 years ago

though in that case I'd prefer if we update the DEBio API to work on ReactionSystems, and just have the reaction_network macro return a ReactionSystem.

I kind of think that's where we should be heading.

We should probably get that settled though before @TorkelE finishes the DEBio rewrite. If we go with making everything based on ReactionSystems I'd suggest maybe keeping most of the functionality in DEBio to keep the exports from MT down. Users who want anything beyond basic ReactionSystem functionality could just import DEBio too.

Totally agree.

BTW, could the BCR network even be loaded right now, or would it hit this GG issue: thautwarm/GeneralizedGenerated.jl#45

eval would at least work, but I am not sure. We'd have to generate it find out. We have some alternative solutions in the pipelines too.

TorkelE commented 4 years ago

Personally I am agnostic towards having the custom types for the reaction_network reaction networks, and I agree that it has some complicating consequences.

I am all for ditching evals wherever they can be found! I agree that this should generate a ReactionSystem (if we want to have reaction_network separate, we could generate one of those from there, but that seems unlikely).

The current version of the DiffEqBio rework is pretty much just a way of generating a ReactionSystem already. The only additional thing it does is wrapping it into another type, and storing it with ode_systems, sde_systems and such, potentially avoid having to re-generate these. It also stores the variables and symbols, as symbol vectors, but I presume these could easily be generated from the ReactionSystem via some function.

I will have a think if I can come up with anything that would belong in the current reaction_network, but not b appropriate in a reaction-system.

isaacsas commented 4 years ago

Sounds good. I think I've got dependency graphs in for MT now. So that should basically give us feature parity with the old DEBio once I update the ReactionSystem to jump conversion routine to use them. Assuming we convert the API to take ReactionSystems instead of reaction_networks we're pretty close to being converted over!

TorkelE commented 4 years ago

Yes. Things have been on halt most of the last week, turned out sitting long hours working from a normal chair is not a good idea and my back started hurting... I have gotten a proper chair for my lockdown home-office now, and things should start to improve.

I think we are quite close to being done. The only hold up is the bifurcation part. Transitioning to PALC for the bifurcation plots seems like the target, but I still uncertain exactly how we should structure this, and I am a bit reluctant to throw away the current methodology without a replacement ready (especially since I use it on a weekly basis).

I am not fully certain, but currently I think that Homotopy Continuation offers unique functionality for solving steady states at individual parameter values, and that it would make sense to keep that. But I am not sure where it would fit in the new approach. With trying to make the DiffEqBio package just a DSL, it doesn't really suit there. But I am not sure if it is possible to split it off into its entire own package, without also introducing an eval call, and all of the implications of that. The third option would be to have a way in MTK for generating a polynomial system from a ReactionSystem, but that would also require some amount of work. Also, I don't think that the current polynomial packages support parameters in a very general sense(?) (That is, parameters "work", but are treated as a variable, and thus you cannot have parameters in exponents, or things like exp(p)*x^2)

isaacsas commented 4 years ago

Why would you require an eval call? It seems like conversion of a ReactionSystem to a HCPolynomial system in MT might be the ideal way to do this, and would have the benefit of letting other people use HC functionality for polynomial systems arising in other contexts. I have no idea though how easy this is do to. Couldn't a HC polynomial system simply require parameters only appear as polynomials too?

TorkelE commented 4 years ago

I don't think a special HCPolynomial would be required, just a normal Polynomial. Then one unlocks all of the Polynomial stuff as well, which contains some good stuff, yes, that would probably be the ideal option.

It would probably be rather possible to make something, which generates a polynomial given a set of parameters, and then one can do parameter stuff with that. But I will check with the people over at polynomials as well, they would know what would and wouldn't work.

isaacsas commented 3 years ago

0.2.0 will generate ReactionSystems so closes this.