Parse error during _get_reaction_system on yeast v9

[oxinabox]

The Yeast v9 GEM: https://raw.githubusercontent.com/SysBioChalmers/yeast-GEM/main/model/yeast-GEM.xml
cause a parse error during _get_reaction_system

julia> prn, cb = load_SBML("yeast-GEM.xml")
ERROR: ParseError:
# Error @ none:1:19
Catalyst.Reaction(, [M_s_2808, M_s_3610], [M_s_2783, M_s_2966, M_s_3586], [1, 1], [2, 1, 1]; metadata = [:name => "PI 4,5-P2 phosphatase (1-18:0, 2-16:1), ER membrane", :id => "R_r_3210"], only_use_rate=true)
#                 └───────────────────┘ ── Expected `)`
Stacktrace:
 [1] #parse#3
   @ ./meta.jl:244 [inlined]
 [2] parse
   @ ./meta.jl:236 [inlined]
 [3] parse(str::SubString{String}; filename::String, raise::Bool, depwarn::Bool)
   @ Base.Meta ./meta.jl:278
 [4] parse
   @ ./meta.jl:276 [inlined]
 [5] _get_reaction_system(model_SBML_sys::SBMLImporter.ModelSBMLSystem, model_SBML::SBMLImporter.ModelSBML)
   @ SBMLImporter ~/.julia/packages/SBMLImporter/5N4Rf/src/system.jl:40
 [6] 
   @ SBMLImporter ~/.julia/packages/SBMLImporter/5N4Rf/src/load.jl:84
 [7] load_SBML(path::String)
   @ SBMLImporter ~/.julia/packages/SBMLImporter/5N4Rf/src/load.jl:74
 [8] top-level scope
   @ ~/repos/POC_kinetic/test/yeast.jl:6
Some type information was truncated. Use `show(err)` to see complete types.

I haven't tested older versions of the GEM.
But I did test SBMLImporter v1.2, Catalyst v13, and that does error in same way.

SBML.jl works fine

julia> using SBML
       mdl = readSBML(model_file_yeast())
SBML.Model with 4130 reactions, 2805 species, and 5 parameters.

Looks like SBMLImporter is generating some julia code as a string, and then parsing it (what a nightmarish thing to do), and the code it is generating is wrong.

I suspect that the rate field is nothing and when interpolated into a string that makes the empty string, and that breaks things.
Not supporting reactions that don't have rates is fairly reasonable.
Ideally though I would kinda like to have them presented to me as parameters with missing defaults that I could go and fill in later with data from other sources.
But non-ideally, I would like an informative error message.

[sebapersson]

Thanks for spotting this, never seen this before!

I also suspect the rate field is nothing. As a default option I do not want to support such reactions (as most often it should indicate the user has done something wrong, but a more informative error should be thrown). But I think it is fair to also generate new parameters on the form of something like: generated_k_reactionid that defaults to zero (the generated part of the id would be to allow users to identify generated parameters), via a keyword argument if the user knows what they are doing (it is also easy to implement).

In summary, I can add a keyword argument to load_SBML for handling these kind of reactions, where new parameters are generated on the form of generated_k_reactionid if sounds good?

SBMLImporter is generating some julia code as a string, and then parsing it (what a nightmarish thing to do :)

You are correct, it is not pretty, but the long-term plan is to transition to Symbolics (but SBML is so strange that this is not so simple with all the possible things that can be inlined recursively into a SBML rate-expression :)

[oxinabox]

In summary, I can add a keyword argument to load_SBML for handling these kind of reactions, where new parameters are generated on the form of generated_k_reactionid if sounds good?

I think that sounds good.
Then I would use SBML.jl directly to access whatever other metadata I need to go from reaction-id to the correct reaction rate.
(Or fit them to data with parameter estimation?)

[sebapersson]

(Or fit them to data with parameter estimation?)

If possible I would try to use available metadata to set parameter values. It is possible to estimate parameters also (e.g. using PEtab.jl which is the reason SBMLImporter exists), but for the Yeast v9 GEM model I think parameter estimation would be to computationally demanding (to my knowledge the largest model parameter estimated in a somewhat good manner is this one https://www.embopress.org/doi/full/10.15252/msb.202210988, and for this model the first author who is very good at these things had to use a lot of tricks)