Merge branch 'develop' into check_grad_sbml_test_suite

Author: dweindl

.github/workflows/test_windows.yml

@@ -8,8 +8,6 @@ on:
   pull_request:
     branches:
       - master
-      # TODO: remove before merge
-      - develop
 
 jobs:
   build:
@@ -87,12 +85,6 @@ jobs:
           --ignore-glob=*test_splines.py \
           python/tests
 
-    # TODO <REMOVE BEFORE MERGE>
-    - name: Python tests splines
-      if: ${{ github.base_ref == 'develop' || github.event.merge_group.base_ref == 'develop'}}
-      run: python -m pytest python/tests/test_splines.py
-    # TODO </REMOVE BEFORE MERGE>
-
     - name: Python tests splines
       if: ${{ github.base_ref == 'master' || github.event.merge_group.base_ref == 'master'}}
       run: python -m pytest python/tests/test_splines.py

CHANGELOG.md

@@ -2,6 +2,38 @@
 
 ## v0.X Series
 
+### v0.18.0 (2023-05-26)
+Features:
+* More efficient handling of splines in SBML models
+  by @paulstapor, @lcontento, @dweindl
+  in https://github.com/AMICI-dev/AMICI/pull/1515
+* Partial support of current PEtab2.0 draft, including support for PySB models
+  by @dweindl, @FFroehlich in https://github.com/AMICI-dev/AMICI/pull/1800
+
+Fixes
+* **Fixed incorrect forward sensitivities for models with events with**
+  **state-dependent trigger functions**
+  by @dweindl in https://github.com/AMICI-dev/AMICI/pull/2084
+* Model import: Don't create spl.h and sspl.h for models without splines
+  by @dweindl in https://github.com/AMICI-dev/AMICI/pull/2088
+* SBML import - faster processing of SpeciesReference IDs
+  by @dweindl in https://github.com/AMICI-dev/AMICI/pull/2094
+* Update swig ignores
+  by @FFroehlich in https://github.com/AMICI-dev/AMICI/pull/2098
+* CMake: Fixed choosing SWIG via `SWIG` env variable
+  by @dweindl in https://github.com/AMICI-dev/AMICI/pull/2100
+* CMake: Try FindBLAS if no other information was provided
+  by @dweindl in https://github.com/AMICI-dev/AMICI/pull/2104
+* Fixed cblas error for models without solver states in combination with
+  forward sensitivities
+  by @dweindl in https://github.com/AMICI-dev/AMICI/pull/2108
+* Fixed compilation error for models with events and xdot=0
+  by @dweindl in https://github.com/AMICI-dev/AMICI/pull/2111
+* Fixed import error for models with events and 0 states
+  by @dweindl in https://github.com/AMICI-dev/AMICI/pull/2112
+
+**Full Changelog**: https://github.com/AMICI-dev/AMICI/compare/v0.17.1...v0.18.0
+
 ### v0.17.1 (2023-05-10)
 
 This release fixes two bugs:

python/sdist/amici/de_export.py

@@ -1993,6 +1993,10 @@ def _compute_equation(self, name: str) -> None:
             ]
 
         elif name == "deltasx":
+            if self.num_states_solver() * self.num_par() == 0:
+                self._eqs[name] = []
+                return
+
             event_eqs = []
             for ie, event in enumerate(self._events):
                 tmp_eq = sp.zeros(self.num_states_solver(), self.num_par())

python/sdist/amici/petab_util.py

@@ -58,6 +58,11 @@ def get_states_in_condition_table(
             return states
         import pysb.pattern
 
+        if not petab_problem.model.model.species:
+            import pysb.bng
+
+            pysb.bng.generate_equations(petab_problem.model.model)
+
         try:
             spm = pysb.pattern.SpeciesPatternMatcher(
                 model=petab_problem.model.model

python/sdist/amici/swig_wrappers.py

@@ -1,6 +1,8 @@
 """Convenience wrappers for the swig interface"""
 import logging
 import sys
+
+import warnings
 from contextlib import contextmanager, suppress
 from typing import Any, Dict, List, Optional, Sequence, Union
 
@@ -102,6 +104,18 @@ def runAmiciSimulation(
     :returns:
         ReturnData object with simulation results
     """
+    if (
+        model.ne > 0
+        and solver.getSensitivityMethod()
+        == amici_swig.SensitivityMethod.adjoint
+        and solver.getSensitivityOrder() == amici_swig.SensitivityOrder.first
+    ):
+        warnings.warn(
+            "Adjoint sensitivity analysis for models with discontinuous right hand sides (events/piecewise functions) has not been thoroughly tested."
+            "Sensitivities might be wrong. Tracked at https://github.com/AMICI-dev/AMICI/issues/18. "
+            "Adjoint sensitivity analysis may work if the location of the discontinuity is not parameter-dependent, but we still recommend testing accuracy of gradients."
+        )
+
     with _capture_cstdout():
         rdata = amici_swig.runAmiciSimulation(
             _get_ptr(solver), _get_ptr(edata), _get_ptr(model)
@@ -152,6 +166,18 @@ def runAmiciSimulations(
 
     :returns: list of simulation results
     """
+    if (
+        model.ne > 0
+        and solver.getSensitivityMethod()
+        == amici_swig.SensitivityMethod.adjoint
+        and solver.getSensitivityOrder() == amici_swig.SensitivityOrder.first
+    ):
+        warnings.warn(
+            "Adjoint sensitivity analysis for models with discontinuous right hand sides (events/piecewise functions) has not been thoroughly tested. "
+            "Sensitivities might be wrong. Tracked at https://github.com/AMICI-dev/AMICI/issues/18. "
+            "Adjoint sensitivity analysis may work if the location of the discontinuity is not parameter-dependent, but we still recommend testing accuracy of gradients."
+        )
+
     with _capture_cstdout():
         edata_ptr_vector = amici_swig.ExpDataPtrVector(edata_list)
         rdata_ptr_list = amici_swig.runAmiciSimulations(

python/tests/test_splines.py

@@ -9,6 +9,7 @@
 import os
 
 import numpy as np
+from amici.testing import skip_on_valgrind
 from splines_utils import (
     check_splines_full,
     example_spline_1,
@@ -17,6 +18,7 @@
 )
 
 
+@skip_on_valgrind
 def test_multiple_splines(**kwargs):
     """
     Test a SBML model containing multiple splines.

python/tests/test_splines_python.py

@@ -9,8 +9,10 @@
 
 import amici
 import sympy as sp
+from amici.testing import skip_on_valgrind
 
 
+@skip_on_valgrind
 def test_SplineUniform():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -27,6 +29,7 @@ def test_SplineUniform():
     assert math.isclose(float(spline.evaluate(1.00)), 1.0)
 
 
+@skip_on_valgrind
 def test_SplineNonUniform():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -43,6 +46,7 @@ def test_SplineNonUniform():
     assert math.isclose(float(spline.evaluate(1.00)), 1.0)
 
 
+@skip_on_valgrind
 def test_SplineExplicit():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -62,6 +66,7 @@ def test_SplineExplicit():
     assert math.isclose(float(spline.evaluate(1.00)), 0.75)
 
 
+@skip_on_valgrind
 def test_SplineZeroBC():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -77,6 +82,7 @@ def test_SplineZeroBC():
     assert math.isclose(float(spline.evaluate(1.00)), 1.0)
 
 
+@skip_on_valgrind
 def test_SplineLogarithmic():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -96,6 +102,7 @@ def test_SplineLogarithmic():
     assert math.isclose(float(spline.evaluate(1.00)), 0.75)
 
 
+@skip_on_valgrind
 def test_SplineUniformConstantExtrapolation():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -117,6 +124,7 @@ def test_SplineUniformConstantExtrapolation():
     assert math.isclose(float(spline.evaluate(3.00)), 1.0)
 
 
+@skip_on_valgrind
 def test_SplineUniformLinearExtrapolation():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -138,6 +146,7 @@ def test_SplineUniformLinearExtrapolation():
     assert math.isclose(float(spline.evaluate(3.00)), 4.0)
 
 
+@skip_on_valgrind
 def test_SplineUniformPolynomialExtrapolation():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -159,6 +168,7 @@ def test_SplineUniformPolynomialExtrapolation():
     assert math.isclose(float(spline.evaluate(3.00)), -248.0)
 
 
+@skip_on_valgrind
 def test_SplineUniformPeriodicExtrapolation():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -180,6 +190,7 @@ def test_SplineUniformPeriodicExtrapolation():
     assert math.isclose(float(spline.evaluate(2.75)), 0.47265625)
 
 
+@skip_on_valgrind
 def test_SplineNonUniformPeriodicExtrapolation():
     spline = amici.splines.CubicHermiteSpline(
         sbml_id="f",
@@ -201,13 +212,15 @@ def test_SplineNonUniformPeriodicExtrapolation():
     assert math.isclose(float(spline.evaluate(2.05)), 1.5296875)
 
 
+@skip_on_valgrind
 def check_gradient(spline, t, params, params_values, expected, rel_tol=1e-9):
     value = spline.evaluate(t)
     subs = {pname: pvalue for (pname, pvalue) in zip(params, params_values)}
     for p, exp in zip(params, expected):
         assert math.isclose(float(value.diff(p).subs(subs)), exp, rel_tol=rel_tol)
 
 
+@skip_on_valgrind
 def test_SplineUniformSensitivity():
     params = (a, b, c) = sp.symbols("a b c")
     params_values = [0.5, 1.0, 2.5]
@@ -230,6 +243,7 @@ def test_SplineUniformSensitivity():
     check_gradient(spline, 1.00, params, params_values, [-6.0, 1.0, 3.0])
 
 
+@skip_on_valgrind
 def test_SplineNonUniformSensitivity():
     params = (a, b, c) = sp.symbols("a b c")
     params_values = [0.5, 1.0, 2.5]
@@ -250,6 +264,7 @@ def test_SplineNonUniformSensitivity():
     check_gradient(spline, 1.00, params, params_values, [-6.0, 1.0, 3.0])
 
 
+@skip_on_valgrind
 def test_SplineExplicitSensitivity():
     params = (a, b, c) = sp.symbols("a b c")
     params_values = [0.5, 1.0, 2.5]
@@ -280,6 +295,7 @@ def test_SplineExplicitSensitivity():
     check_gradient(spline, 1.00, params, params_values, [-6.0, 1.0, 3.0])
 
 
+@skip_on_valgrind
 def test_SplineLogarithmicSensitivity():
     params = (a, b, c) = sp.symbols("a b c")
     params_values = [0.5, 1.0, 2.5]

python/tests/test_splines_short.py

@@ -8,6 +8,7 @@
 import numpy as np
 import sympy as sp
 from amici.splines import CubicHermiteSpline, UniformGrid
+from amici.testing import skip_on_valgrind
 from splines_utils import check_splines_full, example_spline_1
 
 
@@ -20,6 +21,7 @@ def test_spline_piecewise(**kwargs):
     check_splines_full(spline, params, tols, **kwargs)
 
 
+@skip_on_valgrind
 def test_two_splines(**kwargs):
     """
     Test a SBML model containing two splines.
@@ -68,6 +70,7 @@ def test_two_splines(**kwargs):
     check_splines_full(splines, params, tols, check_piecewise=False, **kwargs)
 
 
+@skip_on_valgrind
 def test_splines_plist():
     """
     Test if AMICI's spline implementation

tests/benchmark-models/benchmark_models.yaml

@@ -23,7 +23,7 @@ Borghans_BiophysChem1997:
   llh: 83.3237191357272
   t_sim: 0.005
   t_fwd: 0.5
-  t_adj: 0.1
+  t_adj: 0.2
   note: benchmark collection reference value matches up to sign when applying log10-correction +sum(log(meas*log(10)) / 2
 
 Brannmark_JBC2010:

version.txt

@@ -1 +1 @@
-0.17.1
+0.18.0