Skip to content

Commit 2caaecf

Browse files
committed
Fix SBML math parsing by using sbmlmath instead of sympy.sympify
sympy.sympify(formulaToL3String(...)) applies Python operator semantics to SBML L3 formulas, which silently diverge for constructs like modulo (sign-of-dividend in SBML vs. sign-of-divisor in Python). Parse the SBML MathML directly via sbmlmath instead. Fixes #283
1 parent 09c58ba commit 2caaecf

3 files changed

Lines changed: 23 additions & 25 deletions

File tree

petab/v1/models/sbml_model.py

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import libsbml
1010
import sympy as sp
11-
from sympy.abc import _clash
11+
from sbmlmath import sbml_math_to_sympy
1212

1313
from ..._utils import _generate_path
1414
from ..sbml import (
@@ -283,28 +283,7 @@ def sympify_sbml(sbml_obj: libsbml.ASTNode | libsbml.SBase) -> sp.Expr:
283283
-------
284284
The sympy expression corresponding to ``sbml_obj``.
285285
"""
286-
ast_node = (
287-
sbml_obj
288-
if isinstance(sbml_obj, libsbml.ASTNode)
289-
else sbml_obj.getMath()
290-
)
291-
292-
parser_settings = libsbml.L3ParserSettings(
293-
ast_node.getParentSBMLObject().getModel(),
294-
libsbml.L3P_PARSE_LOG_AS_LOG10,
295-
libsbml.L3P_EXPAND_UNARY_MINUS,
296-
libsbml.L3P_NO_UNITS,
297-
libsbml.L3P_AVOGADRO_IS_CSYMBOL,
298-
libsbml.L3P_COMPARE_BUILTINS_CASE_INSENSITIVE,
299-
None,
300-
libsbml.L3P_MODULO_IS_PIECEWISE,
301-
)
302-
303-
formula_str = libsbml.formulaToL3StringWithSettings(
304-
ast_node, parser_settings
305-
)
306-
307-
return sp.sympify(formula_str, locals=_clash)
286+
return sbml_math_to_sympy(sbml_obj)
308287

309288

310289
def antimony2sbml(ant_model: str | Path) -> str:

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ dependencies = [
2020
"jsonschema",
2121
"antlr4-python3-runtime==4.13.1",
2222
"pydantic>=2.10",
23+
"sbmlmath>=0.4.0",
2324
]
2425
license = "MIT"
2526
authors = [
@@ -64,7 +65,6 @@ doc = [
6465
"ipython>=7.21.0, !=8.7.0",
6566
"pysb",
6667
"antimony>=2.14.0",
67-
"sbmlmath>=0.4.0",
6868
]
6969
vis = [
7070
"matplotlib>=3.6.0",

tests/v1/test_sbml.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import pandas as pd
66
import pytest
77

8-
from petab.v1.models.sbml_model import SbmlModel
8+
from petab.v1.models.sbml_model import SbmlModel, sympify_sbml
99

1010
sys.path.append(os.getcwd())
1111
import petab # noqa: E402
@@ -150,3 +150,22 @@ def test_sbml_from_to_ant():
150150

151151
# convert back to antimony
152152
assert "R1: S1 -> S2; k1*S1" in petab_model.to_antimony()
153+
154+
155+
def test_sympify_sbml_modulo():
156+
"""SBML's ``%`` follows C semantics (result has the sign of the
157+
dividend), unlike Python's ``%`` (result has the sign of the divisor).
158+
159+
Naively converting the SBML math to a string and handing it to
160+
``sympy.sympify`` therefore silently produces a wrong result (gh-283).
161+
"""
162+
sbml_document = libsbml.SBMLDocument(3, 2)
163+
sbml_model = sbml_document.createModel()
164+
parameter = sbml_model.createParameter()
165+
parameter.setId("e")
166+
parameter.setConstant(False)
167+
initial_assignment = sbml_model.createInitialAssignment()
168+
initial_assignment.setSymbol("e")
169+
initial_assignment.setMath(libsbml.parseL3Formula("-5 % 3"))
170+
171+
assert sympify_sbml(initial_assignment).doit() == -2

0 commit comments

Comments
 (0)