From 93885e3992ca5a13a7b5532748ea22f4b9ad67cc Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Tue, 12 Dec 2023 22:01:44 +0100 Subject: [PATCH 1/4] Make test_petab_model.py runnable from any directory (#2233) So far, this file relied on pwd == __file__, changed that. --- tests/benchmark-models/test_petab_model.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/benchmark-models/test_petab_model.py b/tests/benchmark-models/test_petab_model.py index 911c19eaa9..8f52a341c4 100755 --- a/tests/benchmark-models/test_petab_model.py +++ b/tests/benchmark-models/test_petab_model.py @@ -9,6 +9,7 @@ import logging import os import sys +from pathlib import Path import amici import numpy as np @@ -100,7 +101,7 @@ def parse_cli_args(): def main(): """Simulate the model specified on the command line""" - + script_dir = Path(__file__).parent.absolute() args = parse_cli_args() loglevel = logging.DEBUG if args.verbose else logging.INFO logger.setLevel(loglevel) @@ -168,10 +169,7 @@ def main(): times["np"] = sum(problem.parameter_df[petab.ESTIMATE]) - pd.Series(times).to_csv( - f"./tests/benchmark-models/{args.model_name}_benchmark.csv" - ) - + pd.Series(times).to_csv(script_dir / f"{args.model_name}_benchmark.csv") for rdata in rdatas: assert ( rdata.status == amici.AMICI_SUCCESS @@ -201,9 +199,7 @@ def main(): ax.get_figure().savefig(fig_path, dpi=150) if args.check: - references_yaml = os.path.join( - os.path.dirname(__file__), "benchmark_models.yaml" - ) + references_yaml = script_dir / "benchmark_models.yaml" with open(references_yaml) as f: refs = yaml.full_load(f) From 9e26037ff52a1bdd8e60ba376cbf0736a17c45fd Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Wed, 13 Dec 2023 15:33:50 +0100 Subject: [PATCH 2/4] GHA: increased number of optimizations for ExampleSplinesSwameye2003.ipynb (#2237) and use all available cores. This example fails too frequently, e.g. https://github.com/dweindl/AMICI/actions/runs/7193472391/job/19591997110 > Exception: All multistarts failed (n_starts is probably too small)! If this error occurred during CI, just run the workflow again. --- .../example_splines_swameye/ExampleSplinesSwameye2003.ipynb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/examples/example_splines_swameye/ExampleSplinesSwameye2003.ipynb b/python/examples/example_splines_swameye/ExampleSplinesSwameye2003.ipynb index 2a3c113bc7..e8b75e49aa 100644 --- a/python/examples/example_splines_swameye/ExampleSplinesSwameye2003.ipynb +++ b/python/examples/example_splines_swameye/ExampleSplinesSwameye2003.ipynb @@ -101,11 +101,11 @@ "source": [ "# If running as a GitHub action, just do the minimal amount of work required to check whether the code is working\n", "if os.getenv(\"GITHUB_ACTIONS\") is not None:\n", - " n_starts = 15\n", + " n_starts = 25\n", " pypesto_optimizer = pypesto.optimize.FidesOptimizer(\n", " verbose=logging.WARNING, options=dict(maxiter=10)\n", " )\n", - " pypesto_engine = pypesto.engine.SingleCoreEngine()" + " pypesto_engine = pypesto.engine.MultiProcessEngine()" ] }, { From eba9557e04b755a89aec2dd5859f6e6578008832 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Wed, 13 Dec 2023 15:35:34 +0100 Subject: [PATCH 3/4] Set PEtab benchmark collection path via `$BENCHMARK_COLLECTION` (#2236) More convenient to use locally. Avoids having multiple copies of the PEtab benchmark collection lying around for different projects. --- tests/benchmark-models/test_petab_benchmark.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/benchmark-models/test_petab_benchmark.py b/tests/benchmark-models/test_petab_benchmark.py index d0a783e2c4..753c88e500 100755 --- a/tests/benchmark-models/test_petab_benchmark.py +++ b/tests/benchmark-models/test_petab_benchmark.py @@ -1,5 +1,5 @@ """Tests for simulate_petab on PEtab benchmark problems.""" - +import os from pathlib import Path import amici @@ -13,13 +13,13 @@ ATOL: float = 1e-3 RTOL: float = 1e-2 -benchmark_path = ( - Path(__file__).parent.parent.parent - / "Benchmark-Models-PEtab" - / "Benchmark-Models" -) +repo_root = Path(__file__).parent.parent.parent +benchmark_path = repo_root / "Benchmark-Models-PEtab" / "Benchmark-Models" +if not benchmark_path.exists(): + benchmark_path = Path(os.environ["BENCHMARK_COLLECTION"]) + # reuse compiled models from test_benchmark_collection.sh -benchmark_outdir = Path(__file__).parent.parent.parent / "test_bmc" +benchmark_outdir = repo_root / "test_bmc" models = [ str(petab_path.stem) for petab_path in benchmark_path.glob("*") From 594b07e599334f5235c1f70bbddf08446d7a9673 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 14 Dec 2023 10:45:35 +0100 Subject: [PATCH 4/4] Fix piecewise/Heaviside handling (#2234) * Fixes a bug where Heaviside functions weren't correctly substituted, potentially resulting in DiracDelta's in the model equations. The problem was, the substitution targets were collected from an expanded expression, but the actual substitution was attempted on the non-expanded expression, which did not always succeed. Fixed by *not* expanding the expressions beforehand. Closes #2231 * Fixes redundant trigger-function processing https://github.com/AMICI-dev/AMICI/pull/2234 --- python/sdist/amici/de_export.py | 10 +++------- python/sdist/amici/import_utils.py | 15 +++++++++++++++ python/sdist/amici/sbml_import.py | 2 +- tests/benchmark-models/test_petab_benchmark.py | 0 4 files changed, 19 insertions(+), 8 deletions(-) mode change 100755 => 100644 tests/benchmark-models/test_petab_benchmark.py diff --git a/python/sdist/amici/de_export.py b/python/sdist/amici/de_export.py index ac857b366c..2694f753ad 100644 --- a/python/sdist/amici/de_export.py +++ b/python/sdist/amici/de_export.py @@ -58,8 +58,8 @@ generate_flux_symbol, smart_subs_dict, strip_pysb, - symbol_with_assumptions, toposort_symbols, + unique_preserve_order, ) from .logging import get_logger, log_execution_time, set_log_level @@ -2745,16 +2745,12 @@ def _process_heavisides( :returns: dxdt with Heaviside functions replaced by amici helper variables """ - - # expanding the rhs will in general help to collect the same - # heaviside function - dt_expanded = dxdt.expand() # track all the old Heaviside expressions in tmp_roots_old # replace them later by the new expressions heavisides = [] # run through the expression tree and get the roots - tmp_roots_old = self._collect_heaviside_roots(dt_expanded.args) - for tmp_old in tmp_roots_old: + tmp_roots_old = self._collect_heaviside_roots(dxdt.args) + for tmp_old in unique_preserve_order(tmp_roots_old): # we want unique identifiers for the roots tmp_new = self._get_unique_root(tmp_old, roots) # `tmp_new` is None if the root is not time-dependent. diff --git a/python/sdist/amici/import_utils.py b/python/sdist/amici/import_utils.py index 77a2add60b..2dfc60d0c5 100644 --- a/python/sdist/amici/import_utils.py +++ b/python/sdist/amici/import_utils.py @@ -734,5 +734,20 @@ def strip_pysb(symbol: sp.Basic) -> sp.Basic: return symbol +def unique_preserve_order(seq: Sequence) -> list: + """Return a list of unique elements in Sequence, keeping only the first + occurrence of each element + + Parameters: + seq: Sequence to prune + + Returns: + List of unique elements in ``seq`` + """ + seen = set() + seen_add = seen.add + return [x for x in seq if not (x in seen or seen_add(x))] + + sbml_time_symbol = symbol_with_assumptions("time") amici_time_symbol = symbol_with_assumptions("t") diff --git a/python/sdist/amici/sbml_import.py b/python/sdist/amici/sbml_import.py index dd24b98cf8..b6a7b37ed4 100644 --- a/python/sdist/amici/sbml_import.py +++ b/python/sdist/amici/sbml_import.py @@ -38,7 +38,6 @@ DEModel, _default_simplify, smart_is_zero_matrix, - symbol_with_assumptions, ) from .import_utils import ( RESERVED_SYMBOLS, @@ -54,6 +53,7 @@ sbml_time_symbol, smart_subs, smart_subs_dict, + symbol_with_assumptions, toposort_symbols, ) from .logging import get_logger, log_execution_time, set_log_level diff --git a/tests/benchmark-models/test_petab_benchmark.py b/tests/benchmark-models/test_petab_benchmark.py old mode 100755 new mode 100644