search

draeger-lab / ModelPolisher

ModelPolisher accesses the BiGG Models knowledgebase to annotate SBML models.
MIT License
23 stars 7 forks source link

Remove objective if SBML has an objective, but no listOfFluxObjectives. #47

Closed codekaust closed 2 years ago

codekaust commented 5 years ago

This issue extends from https://github.com/SBRG/bigg_models/issues/333.

In models like iAT_PLT_636, an objective is present but no listOfObjectives is there.

Add feature in ModelPolisher to remove objective function in such cases.

mephenor commented 5 years ago

As discussed, I am not sure if this is due to some changes in JSBML or ModelPolisher, but no empty listOfObjectives is present for the models I've tested with the current version. Once I've run it on all bigg_models, I'll report back

draeger commented 5 years ago

@mephenor What is the current state of your tests?

mephenor commented 5 years ago

Wanted to finish DockerHub integration first and then check all models with the release version and compare with the models present in http://bigg.ucsd.edu/, as I forgot to check that when I ran ModelPolisher before release and removed the output directory in the meantime.

mephenor commented 4 years ago

There is some code in the SBMLFix class that should deal with this, I'll just need to have a proper look at it, to see whether it does this correctly for all cases or not. This currently also pertains to https://github.com/SBRG/bigg_models/issues/366.

draeger commented 4 years ago

And we need to be careful when removing information like this from a model - it should only be done when absolutely necessary for ensuring the validity of the file.

mephenor commented 4 years ago

Both COBRAparser and JSONparser set flux objectives and thus add a default objective when parsing, but SBMLpolisher does not do this. I added code to convert the OBJECTIVE_COEFFICIENT from a reaction kinetic law in the same manner, i.e. as long as there is a KineticLaw with OBJECTIVE_COEFFICIENT != 0, the objective should be existing and the list should contain at least one element. Thus an empty listOfFluxObjectives can only exist now, if no fluxObjectives can be created, so we should be able to safely remove an empty objective. Added code to do this to SBMLPolisher.

Schmoho commented 2 years ago

I just realized that unfortunately there is a bug in org.sbml.jsbml.ext.fbc.ListOfObjectives where when the active objective is deleted, returning the instance gives null, but the isSetActiveObjective flag is still true. https://github.com/sbmlteam/jsbml/issues/250

I added a unit test to check that the MP does remove the empty objectives, fixed a bug where this failed when the list of flux objectives was not null but empty.

Closing this as tested.