the way we check for FBC strict is not compliant with the spec

[Schmoho]

From the spec section 3.3 page 8 (line numbers at the end):

This is accomplished by defining a set of restrictions which come into effect if strict is set to “true”: 7
■ Each Reaction in a Model must define attributes lowerFluxBound and upperFluxBound with each pointing 8
to a valid Parameter object defined in the current Model. 9
■ Each Parameter object referred to by the Reaction attributes lowerFluxBound and upperFluxBound must 10
have their constant attribute set to “true” and its value attribute set to a double value which may not be 11
“NaN”. 12
■ SpeciesReference elements of Reactions must have their stoichiometry attribute set to a double value that 13
is neither “NaN” nor “-INF” nor “INF”. In addition their constant attribute must be set to “true”. 14
■ InitialAssignment elements may neither target the Parameter elements referenced by the Reaction attributes 15
lowerFluxBound and upperFluxBound nor any SpeciesReference. 16
■ All defined FluxObjective elements must have their coefficient attribute set to a double value that is nei- 17
ther “NaN” nor “-INF” nor “INF”. 18
■ A Reaction lowerFluxBound attribute may not point to a Parameter with a value of “INF”. 19
■ A Reaction upperFluxBound attribute may not point to a Parameter with a value of “-INF”. 20
■ For all reactions, the value of a lowerFluxBound must be less than or equal to the value of the upperFluxBound. 21

Instead, what we are checking right now is:

• does any InitialAssignment have parameters that carry an SBO term which indicates flux bounds (625) (here)
• ensure all reactions have a lower and upper bound (here)
• are the bounds not INF, NaN and correctly ordered (lines 10-12, 19, 20, 21) (same place)
• do all species references have a non-NaN, non-INF value for their stoichiometry (lines 13,14)
• check if all flux objectives are finite-non-NaN (here)

We are not explicitly checking lines 15 and 16, and the SBO-term check we do is not required by the spec.

[matthiaskoenig]

Why is modelpolisher checking this at all? Should this not be done by the SBML validation? I.e. only check if the model is valid and what the validation warnings/errors are without any additional own checking.

[Schmoho]

In the end, the strict attribute is set on the model according to this inference.
I guess the idea might be to allow to set strictness where possible - I assume the validator would not tell you if a model "could" be strict but isn't, right?

However I can't really say if this is actually a sensible thing to do, or to place within the scope of the Polisher.
I.e. @draeger what's your take on this?

[matthiaskoenig]

So if this is just checking if strict can be set I would do the following:

• validate model with strict=False
• if model is valid with strict=False, set strict=True and check if the model is still valid (i.e., does not have errors)
  I.e. use a validation strategy instead of some heuristics.

[draeger]

Exactly, ModelPolisher is not a model validator. It aims to set missing attributes to suitable values. It could, for instance, load a model that is invalid in the sence that the strict attribute isn't defined at all and then give it a value. The validation-based approach @matthiaskoenig suggests makes sense. The SBO term check is a nice addition. I assume ModelPolisher would also try to set those terms if undefined.