COBREXA / COBREXA.jl

COnstraint Based Reconstruction and EXascale Analysis (in Julia)
https://cobrexa.github.io/COBREXA.jl/stable/
Apache License 2.0
9 stars 0 forks source link

Saving StandardModel makes a model in wrong format #62

Closed HettieC closed 1 month ago

HettieC commented 1 month ago

Loading and converting an SBML or JSON model to a StandardModel and then saving the model as either SBML or JSON doesn't produce a proper SBML or JSON file.

using JSONFBCModels, COBREXA
import AbstractFBCModels.CanonicalModel as CM
download_model(
    "http://bigg.ucsd.edu/static/models/e_coli_core.json",
    "e_coli_core.json",
    "7bedec10576cfe935b19218dc881f3fb14f890a1871448fc19a9b4ee15b448d8",
)
model1 = convert(CM.Model,load_model("e_coli_core.json"))
save_model(model1,"ecoli_core_saved.json")

The saved model cannot be opened again.

exaexa commented 1 month ago

Hi! You seem to expect that the "canonical" model format is going to convert itself automatically to JSON, unfortunately that's not the case.

You can "fix" this in two ways, first the save_model call writes out a "serialized" canonical model which can be loaded easily, you just need to tell the system that it should read the canonical model (it expects a JSON one from the extension):

julia> save_model(model1,"ecoli_core_saved.xxx")

julia> load_model(CM.Model, "ecoli_core_saved.xxx")
AbstractFBCModels.CanonicalModel.Model(
  reactions = Dict{String, AbstractFBCModels.CanonicalModel.Reaction}("ACALD" =…
  metabolites = Dict{String, AbstractFBCModels.CanonicalModel.Metabolite}("glu_…
  genes = Dict{String, AbstractFBCModels.CanonicalModel.Gene}("b4301" => Abstra…
  couplings = Dict{String, AbstractFBCModels.CanonicalModel.Coupling}(),
)

As the second option, you probably want to do the conversion before saving so that you actually have an interoperable JSON model in the end:

julia> save_model(convert(JSONFBCModels.JSONFBCModel, model1), "test.json")

julia> load_model("test.json")
JSONFBCModels.JSONFBCModel(#= 95 reactions, 72 metabolites =#)

I think we could add some type argument to do the conversion more easily... perhaps convert_save_model so that you explicitly ask for conversion and there's no default guessing. Will leave this open for a moment for thinking.

@stelmo opinion welcome

HettieC commented 1 month ago

Converting first works, thanks!

exaexa commented 1 month ago

@HettieC would save_converted_model from #63 make sense as a simplification for this case?

HettieC commented 1 month ago

I'd say so yeah, especially as v1 could save a StandardModel without problems :)