Skip to content

Commit

Permalink
Fix piecewise/Heaviside handling (#2234)
Browse files Browse the repository at this point in the history
* 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

#2234
  • Loading branch information
dweindl authored Dec 14, 2023
1 parent eba9557 commit 594b07e
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 8 deletions.
10 changes: 3 additions & 7 deletions python/sdist/amici/de_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down
15 changes: 15 additions & 0 deletions python/sdist/amici/import_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
2 changes: 1 addition & 1 deletion python/sdist/amici/sbml_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
DEModel,
_default_simplify,
smart_is_zero_matrix,
symbol_with_assumptions,
)
from .import_utils import (
RESERVED_SYMBOLS,
Expand All @@ -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
Expand Down
Empty file modified tests/benchmark-models/test_petab_benchmark.py
100755 → 100644
Empty file.

0 comments on commit 594b07e

Please sign in to comment.