[API] User defined constraints

[akaviaLab]

I was looking at updating #996 (see #1228), and it seems like most of the code there is irrelevant.
There is

• a lot of code that duplicates optlang (can be removed)
• code that processes the UserDefinedConstraints to check if they already in the model, and if not make new ones.

What we need is something that would check if a constraint already exists in a model, and if not makes a new one. It seems like there are two options for that

1. Similar to what the code is currently doing. Add constraints to the model, while checking if they exist or not. Constraints are a special class to avoid conflicting with existing variables and constraints.
   ` def add_user_defined_constraints(self, constraints: list) -> None:
   """Adds User defined constraints (FBC V3) to the model

    Parameters
    ----------
    constraints: list
    """
        self._const_ids.add(constraint.id)
        constraint._model = self
        list_of_var_cons = []
   
        for item in constraint.constraint_comps:
            if item.variable in self.reactions:
                rxn = self.reactions.get_by_id(item.variable)
                var_to_add = rxn.flux_expression
            else:
                var_to_add = self.problem.Variable(item.variable)
                if var_to_add not in self.variables:
                    list_of_var_cons.append(var_to_add)
   
            # add the variable/flux to the constraint expression
            cons_exp += item.coefficient * pow(var_to_add, var_pow)
   
        # make the constraint using expression and bounds
        new_constraint = self.problem.Constraint(
            cons_exp,
            name=constraint.id,
            lb=constraint.lower_bound,
            ub=constraint.upper_bound,
        )
   
        list_of_var_cons.append(new_constraint)
        self.user_defined_const.append(constraint)
        self.add_cons_vars(list_of_var_cons)
   

`

2. Add a method to Model, which would look like
   def UserVariable(self, id): return self.variables.get(id, None) or Variable(id)

Add a similar method for constraint, with the possiblity of overwriting. Something like
def UserConstraint(self, id, dict): const = self.constraints.get(id, None) or Constraint(id) for property in dict: setattr(const, property, dict[property])

Perhaps keep a very simplified version of add_user_constraints.
I think 2 could be cleaner, but I'm not sure. Any comments from the developers? A third option?

---

Both methods would require import/export and seeing that it confirms with FBC3, so that wouldn't be different between them.

[cdiener]

Hi, thanks for opening the discussion. COBRAPY already allows for user defined constraints, and those include all constraints captured by the FBC proposal. So I am not sure if we need a new constraint type. We don't really distinguish between steady state constraints and constraints added by the user afterwards, or constraints working on forward fluxes of forward and reverse fluxes. So the only thing really needed is a filter function that identifies all constraints compatible with FBC considers user-defined constraints and then a function that write those constraints to SBML/JSON.

The filter is the following if I remember the spec correctly:

• the constraint is not a metabolite steady-state condition (those are named like the metabolites in cobrapy so easy to identify)
• the constraint is not variable lower/upper bound (don't appear in constraints anyway)
• the constraint includes v_forward and v_reverse with the same coefficient with opposing signs

The constraints that pass the filter should contain all the information required to write them to SBML. There may be issues with the IDs like with metabolites/reactions.

[akaviaLab]

Leave constraint annotations for now, okay. I'll add a TODO for this when I work on constraint PR.

JSON - which version of JSON do we want to use? The schema is pretty old? Also, how detailed do we want to be?

Say the minimum about a model

• has metabolites
• has reactions

Or the maximum
-has metabolites, which are a dict that must contain id, can contain name, charge
-- Allowable pattern for id is ???? Allowable type for charge is int/float,

• has reactions, which are a dict that contains metabolites, must contain id, etc.
  --- Allowable pattern for id is ???? for name is????
• groups
• genes
• model must have at least one reaction, genes and groups are optional

I don't understand this section at all
"For the parameters, that is currently handled during parsing. So the parameters are substituted by their values. If those appear in the constraint expression, they will be subtracted from the bounds, so x + c < 2 will become x < 2 - c. This is how it is also handled with bound parameters right now in SBML. Non-constant parameters are currently not supported."
But I will do the constraints including reading/writing in a new PR, and we can see if it works.

[cdiener]

I don't understand this section at all

This is the reply to your question regarding section 4.4. SBML has parameters which in cobrapy will just be substituted with their values on parsing. Or maybe I misunderstood your question there? The constraints in libsbml are just linear combinations of coefficients and variables just like in optlang...

[cdiener]

For JSON I would start from the v2 spec in the FBC3 PR. Might make sense to add the new extended spec as v3. The ID constraints would be a good separate discussion. There is a slight incompatibility between cobrapy (id can be any string) and SBML (has formatting constraints for IDs).

[akaviaLab]

For JSON I would start from the v2 spec in the FBC3 PR. Might make sense to add the new extended spec as v3. The ID constraints would be a good separate discussion. There is a slight incompatibility between cobrapy (id can be any string) and SBML (has formatting constraints for IDs).

Do you think we should stay with JSON schema draft 4? It is pretty old - see all schemas and when they were released at https://json-schema.org/specification-links.html

[cdiener]

Oh, I see you meant the JSON schema version itself. Feel free to update this one to the latest version.

[akaviaLab]

Look at #1228, which has a function to add user constraints, and example test cases, while removing most of the unique code. It needed some unique functions to make sure we don't add the same Variable or Constraint multiple times, which lead to crashes. Perhaps I could have rewritten other functions (in optlang), but that's what I did, updating the original. If you look at #1228, I just need some help with one of the tests - the optlang version doesn't give the same results as the complicated old version. Can you point me to the standard of constraints for FBC/SBML? I don't know what it is based on. I have functions that write to JSON, and if #1228 is good, I can write functions for SBML if I get the standard.

…

On Fri, Jul 1, 2022 at 3:32 PM Christian Diener ***@***.***> wrote: Hi, thanks for opening the discussion. COBRAPY already allows for user defined constraints, and those include all constraints captured by the FBC proposal. So I am not sure if we need a new constraint type. We don't really distinguish between steady state constraints and constraints added by the user afterwards, or constraints working on forward fluxes of forward and reverse fluxes. So the only thing really needed is a filter function that identifies all constraints compatible with FBC considers user-defined constraints and then a function that write those constraints to SBML/JSON. The filter is the following if I remember the spec correctly: - the constraint is not a metabolite steady-state condition (those are named like the metabolites in cobrapy so easy to identify) - the constraint is not variable lower/upper bound (don't appear in constraints anyway) - the constraint includes v_forward and v_reverse with the same coefficient with opposing signs The constraints that pass the filter should contain all the information required to write them to SBML. There may be issues with the IDs like with metabolites/reactions. — Reply to this email directly, view it on GitHub <#1240 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACQYYZSSLLYWGOJCRV4JGALVR5BWDANCNFSM5Z7GVFDQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[cdiener]

I looked at #1228 but it still handles the user defined constraints separately, for instance by having a separate add and remove method on the Model object.

[cdiener]

See here section 4.4 for the spec.

[cdiener]

Optlang should already disallow adding the same constraint twice, otherwise we should fix it there.

[akaviaLab]

Optlang disallows adding an existing variable or constraint by crashing when you try to optimize, not when adding them. Should it 1) check if the variable exists and then replace the new one with existing? There are functions in #1228 that do that, even if it is the wrong field in Model. 2) give an error/warning and tell you how to get the existing variable. 3) something else?

…

On Fri., Jul. 1, 2022, 3:42 p.m. Christian Diener, ***@***.***> wrote: Optlang should already disallow adding the same constraint twice, otherwise we should fix it there. — Reply to this email directly, view it on GitHub <#1240 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACQYYZRXS5SSBAIRCHEEBOTVR5CZ5ANCNFSM5Z7GVFDQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[cdiener]

Okay, that should be fixed in add_cons_vars then.

[cdiener]

It should give an error. Can't add multiple constraints with the same ID.

[akaviaLab]

Can I add a small function to Model which will be like this code?

@def Variable(name):
   return (self.variables.get(name, None) or Variable(name))

That way, a user can write Variable and if it exists, it will be used. If not, it will be created, and users don't need to understand optlang.

[cdiener]

Something like this would go into optlang not COBRAPY, however I would say this is already handled by the "sloppy" options in optlang which can check whether variables in constraints already exist and then use previously defined ones.

[cdiener]

One thing to keep in mind that this will only be useful after libsbml implements the spec from the design document which may take a while. The design doc for FBC3 is from 2020 but it does not seem to have been implemented in libsbml yet.

[matthiaskoenig]

see below. Everything already implemented in libsbml

[cdiener]

Oh cool, can you paste a link to the docs for the FBC constraints in the Python interface. I was somehow not able to find it.

[matthiaskoenig]

Unfortunately, the docs are not too good. I normally just look at the SBML specification (latest FBC3 is here
https://raw.githubusercontent.com/bgoli/sbml-fbc-spec/main/sf_svn/spec/main.pdf) and then get the objects via the getters. You SHOULD use type annotations then you get very nice documentation directly clicking on the python objects. I.e. the docstrings are amazing, but there is to my knowledge not a nice page for the API documentation.

Working a bit on an example. See below for code
https://github.com/matthiaskoenig/sbmlutils/blob/rules/src/sbmlutils/fbc/fbc_version3.py

Creating the following SBML

<?xml version="1.0" encoding="UTF-8"?>
<sbml xmlns="http://www.sbml.org/sbml/level3/version1/core" xmlns:fbc="http://www.sbml.org/sbml/level3/version1/fbc/version3" level="3" version="1" fbc:required="false">
  <model fbc:strict="true">
    <listOfCompartments>
      <compartment id="c1" size="1" constant="true"/>
    </listOfCompartments>
    <listOfSpecies>
      <species id="s1" compartment="c1" initialConcentration="1" hasOnlySubstanceUnits="false" boundaryCondition="false" constant="true">
        <annotation>
          <listOfKeyValuePairs xmlns="http://www.sbml.org/sbml/level3/version1/fbc/version3">
            <keyValuePair key="testdata" value="1.0"/>
          </listOfKeyValuePairs>
        </annotation>
      </species>
      <species id="s2" compartment="c1" initialConcentration="1" hasOnlySubstanceUnits="false" boundaryCondition="false" constant="true"/>
    </listOfSpecies>
    <listOfParameters>
      <parameter id="lb" value="-100" constant="true"/>
      <parameter id="ub" value="100" constant="true"/>
    </listOfParameters>
    <listOfReactions>
      <reaction id="r1" reversible="true" fast="false">
        <listOfReactants>
          <speciesReference species="s1" stoichiometry="1" constant="true"/>
        </listOfReactants>
        <listOfProducts>
          <speciesReference species="s2" stoichiometry="1" constant="true"/>
        </listOfProducts>
      </reaction>
    </listOfReactions>
    <fbc:listOfUserDefinedConstraints>
      <fbc:userDefinedConstraint fbc:id="constraint1" fbc:lowerBound="lb" fbc:upperBound="ub">
        <fbc:listOfUserDefinedConstraintComponents>
          <fbc:userDefinedConstraintComponent fbc:coefficient="10" fbc:variable="r1" fbc:variableType="linear"/>
        </fbc:listOfUserDefinedConstraintComponents>
      </fbc:userDefinedConstraint>
    </fbc:listOfUserDefinedConstraints>
  </model>
</sbml>

[cdiener]

Thanks the example python code is what I was looking for. It looks like this could be generated directly from optlang constraints passing the aforementioned filters.

[matthiaskoenig]

Yes, everything is implemented in libsbml and the pull request included the writing/reading of the constraints in SBML. I.e. the pull request did not only included the user constraint but also the complete serialization to/from SBML.
I agree annotations/notes can be skipped for now. This is probably not the most important thing here.

[akaviaLab]

Leave constraint annotations for now. I'll add a TODO to note them in the metadata branch. JSON - which version of JSON do we want to use? The schema is pretty old? Also, how detailed do we want to be? Say the minimum about a model - has metabolites - has reactions Or the maximum -has metabolites, which are a dict that must contain id, can contain name, charge -- Allowable pattern for id is ???? Allowable type for charge is int/float, - has reactions, which are a dict that contains metabolites, must contain id, etc. --- Allowable pattern for id is ???? for name is???? - groups - genes - model must have at least one reaction, genes and groups are optional I don't understand this section at all "For the parameters, that is currently handled during parsing. So the parameters are substituted by their values. If those appear in the constraint expression, they will be subtracted from the bounds, so x + c < 2 will become x < 2 - c. This is how it is also handled with bound parameters right now in SBML. Non-constant parameters are currently not supported." But I will do the constraints including reading/writing in a new PR, and we can see if it works.

…

On Tue, Jul 5, 2022 at 4:56 PM Christian Diener ***@***.***> wrote: Good catch with the annotations. This would not be supported for now since optlang objects have no annotations. We probably would not want to implement SBML-style annotations in optlang since that is more general purpose. Maybe just some dict annotations? We could also store a dict of constraint annotations in the model, but this would have to be kept in sync... I would probably leave the constraint annotations out for now and add them in a separate PR/branch after some discussion. I agree that JSON will need a schema entry for the constraints. Since the idea for the new JSON schema was to comply to SBML but in JSON-format, it would probably have the same structure as in SBML. So it would only track the additional constraints. For the parameters, that is currently handled during parsing. So the parameters are substituted by their values. If those appear in the constraint expression, they will be subtracted from the bounds, so x + c < 2 will become x < 2 - c. This is how it is also handled with bound parameters right now in SBML. Non-constant parameters are currently not supported. — Reply to this email directly, view it on GitHub <#1240 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACQYYZWIV2SDK3Q3HHYIJCDVSSON5ANCNFSM5Z7GVFDQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>