Remove mathml2sympy

For spline handling, replace `mathml2sympy` by sbmlmath functions.
Related to #2146.

Also fix potential code printing issues resulting in unintended integer division specifically for splines.

Author: dweindl

pytest.ini

@@ -28,6 +28,7 @@ filterwarnings =
     ignore:divide by zero encountered in log:RuntimeWarning
     # ignore jax deprecation warnings
     ignore:jax.* is deprecated:DeprecationWarning
-
+    # swig 4.3 issues
+    ignore:builtin type .* has no __module__ attribute:DeprecationWarning
 
 norecursedirs = .git amici_models build doc documentation models ThirdParty amici sdist examples *build* petab_test_problems pysb_test_models

python/sdist/amici/exporters/sundials/de_export.py

@@ -838,21 +838,19 @@ def _get_create_splines_body(self):
         body = ["return {"]
         for ispl, spline in enumerate(self.model._splines):
             if isinstance(spline.nodes, splines.UniformGrid):
-                nodes = (
-                    f"{ind8}{{{spline.nodes.start}, {spline.nodes.stop}}}, "
-                )
+                start = self._code_printer.doprint(spline.nodes.start)
+                stop = self._code_printer.doprint(spline.nodes.stop)
+                nodes = f"{ind8}{{{start}, {stop}}}, "
             else:
-                nodes = f"{ind8}{{{', '.join(map(str, spline.nodes))}}}, "
+                nodes = f"{ind8}{{{', '.join(map(self._code_printer.doprint, spline.nodes))}}}, "
 
             # vector with the node values
-            values = (
-                f"{ind8}{{{', '.join(map(str, spline.values_at_nodes))}}}, "
-            )
+            values = f"{ind8}{{{', '.join(map(self._code_printer.doprint, spline.values_at_nodes))}}}, "
             # vector with the slopes
             if spline.derivatives_by_fd:
                 slopes = f"{ind8}{{}},"
             else:
-                slopes = f"{ind8}{{{', '.join(map(str, spline.derivatives_at_nodes))}}},"
+                slopes = f"{ind8}{{{', '.join(map(self._code_printer.doprint, spline.derivatives_at_nodes))}}},"
 
             body.extend(
                 [

python/sdist/amici/sbml_import.py

@@ -1607,7 +1607,10 @@ def _process_rule_assignment(self, rule: libsbml.AssignmentRule):
                     ):
                         raise NotImplementedError(
                             "AMICI at the moment does not support splines "
-                            "whose evaluation point is not the model time."
+                            "whose evaluation point is not the model time. "
+                            f"'evaluate_at' was {spline.evaluate_at} "
+                            f"({type(spline.evaluate_at)}) in spline for "
+                            f"symbol {sym_id}."
                         )
                     self.splines.append(spline)
                     return
@@ -2274,6 +2277,9 @@ def _make_initial(
                     raise NotImplementedError(
                         "AMICI at the moment does not support splines "
                         "whose evaluation point is not the model time."
+                        f"'evaluate_at' was {spline.evaluate_at} "
+                        f"({type(spline.evaluate_at)}) in spline for "
+                        f"symbol {var}."
                     )
                 sym_math = sym_math.subs(
                     var, spline.evaluate(sbml_time_symbol)

python/sdist/amici/sbml_utils.py

@@ -11,8 +11,6 @@
 import sympy as sp
 
 if TYPE_CHECKING:
-    from typing import Any
-
     SbmlID = str | sp.Symbol
 
 import xml.dom.minidom
@@ -23,8 +21,6 @@
 
 from .import_utils import (
     SBMLException,
-    _check_unsupported_functions,
-    _parse_special_functions,
     amici_time_symbol,
     sbml_time_symbol,
 )
@@ -399,6 +395,7 @@ def pretty_xml(ugly_xml: str) -> str:
     return pretty_xml[pretty_xml.index("\n") + 1 :]
 
 
+# TODO: replace by sbmlmath.SBMLMathMLPrinter
 class MathMLSbmlPrinter(MathMLContentPrinter):
     """Prints a SymPy expression to a MathML expression parsable by libSBML.
 
@@ -485,57 +482,3 @@ def set_sbml_math(obj: libsbml.SBase, expr, **kwargs) -> None:
             f"expression:\n{expr}\n"
             f"MathML:\n{pretty_xml(mathml)}"
         )
-
-
-# TODO: replace by `sbmlmath` functions
-def mathml2sympy(
-    mathml: str,
-    *,
-    evaluate: bool = False,
-    locals: dict[str, Any] | None = None,
-    expression_type: str = "mathml2sympy",
-) -> sp.Basic:
-    ast = libsbml.readMathMLFromString(mathml)
-    if ast is None:
-        raise ValueError(
-            f"libSBML could not parse MathML string:\n{pretty_xml(mathml)}"
-        )
-
-    formula = _parse_logical_operators(libsbml.formulaToL3String(ast))
-
-    if evaluate:
-        expr = sp.sympify(formula, locals=locals)
-    else:
-        with sp.core.parameters.evaluate(False):
-            expr = sp.sympify(formula, locals=locals)
-
-    expr = _parse_special_functions(expr)
-
-    if expression_type is not None:
-        _check_unsupported_functions(expr, expression_type)
-
-    return expr
-
-
-# TODO: remove after getting rid of `mathml2sympy`
-def _parse_logical_operators(
-    math_str: str | float | None,
-) -> str | float | None:
-    """
-    Parses a math string in order to replace logical operators by a form
-    parsable for sympy
-
-    :param math_str:
-        str with mathematical expression
-    :param math_str:
-        parsed math_str
-    """
-    if not isinstance(math_str, str):
-        return math_str
-
-    if " xor(" in math_str or " Xor(" in math_str:
-        raise SBMLException(
-            "Xor is currently not supported as logical operation."
-        )
-
-    return (math_str.replace("&&", "&")).replace("||", "|")

python/sdist/amici/splines.py

@@ -48,7 +48,6 @@
     add_assignment_rule,
     add_parameter,
     get_sbml_units,
-    mathml2sympy,
     pretty_xml,
     sbml_mathml,
 )
@@ -1216,9 +1215,9 @@ def from_annotation(
         """Create a spline object from a SBML annotation.
 
         This function extracts annotation and children from the XML annotation
-        and gives them to the ``_fromAnnotation`` function for parsing.
-        Subclass behaviour should be implemented by extending
-        ``_fromAnnotation``.
+        and gives them to the ``_from_annotation`` function for parsing.
+        Subclass behavior should be implemented by extending
+        ``_from_annotation``.
         However, the mapping between method strings and subclasses
         must be hard-coded into this function here (at the moment).
         """
@@ -1240,6 +1239,12 @@ def from_annotation(
                 value = False
             attributes[key] = value
 
+        from sbmlmath import SBMLMathMLParser, TimeSymbol
+
+        mathml_parser = SBMLMathMLParser(
+            symbol_kwargs={"real": True}, ignore_units=True, evaluate=False
+        )
+
         children = {}
         for child in annotation:
             if not child.tag.startswith(f"{{{annotation_namespace}}}"):
@@ -1248,11 +1253,8 @@ def from_annotation(
                 )
             key = child.tag[len(annotation_namespace) + 2 :]
             value = [
-                mathml2sympy(
-                    ET.tostring(gc).decode(),
-                    evaluate=False,
-                    locals=locals_,
-                    expression_type="Rule",
+                mathml_parser.parse_str(ET.tostring(gc).decode()).replace(
+                    TimeSymbol, lambda *args: sbml_time_symbol
                 )
                 for gc in child
             ]

python/tests/test_cxxcodeprinter.py

@@ -62,3 +62,13 @@ def test_min_max():
     assert cp.doprint(sp.Max(a, b)) == "std::max(a, b)"
     assert cp.doprint(sp.Min(a, b, c)) == "std::min({a, b, c})"
     assert cp.doprint(sp.Max(a, b, c)) == "std::max({a, b, c})"
+
+
+@skip_on_valgrind
+def test_float_arithmetic():
+    """
+    Check that AmiciCxxCodePrinter produces code that uses float arithmetic.
+    """
+    cp = AmiciCxxCodePrinter()
+    assert cp.doprint(sp.Rational(1, 2)) == "1.0/2.0"
+    assert cp.doprint(sp.Integer(1) / sp.Integer(2)) == "1.0/2.0"

tests/performance/test.py

@@ -8,6 +8,7 @@
 from pathlib import Path
 
 import amici
+import amici.exporters.sundials.de_export
 import petab.v1 as petab
 from amici.petab.petab_import import import_model_sbml