Skip to content

Commit 72a7a92

Browse files
authored
DEModel._expr_is_time_dependent not accounting for event assignment targets (#3047)
Fixes #3046. Relax the faulty check. Better to track more root functions than missing some. Will optimize after #3031.
1 parent baed9cd commit 72a7a92

2 files changed

Lines changed: 22 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ See also our [versioning policy](https://amici.readthedocs.io/en/latest/versioni
105105
can now be specified via the `AMICI_MODELS_ROOT` environment variable.
106106
See `amici.get_model_dir` for details.
107107

108+
**Fixes**
109+
110+
* Fixed a bug that potentially results in incorrect handling of
111+
discontinuities their location depends on a state variable
112+
that is subject to an event assignment but otherwise constant.
108113

109114
## v0.X Series
110115

python/sdist/amici/de_model.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2236,26 +2236,31 @@ def conservation_law_has_multispecies(self, tcl: ConservationLaw) -> bool:
22362236
def _expr_is_time_dependent(self, expr: sp.Expr) -> bool:
22372237
"""Determine whether an expression is time-dependent.
22382238
2239+
This function is solely for checking whether `expr` has a discontinuity
2240+
we need to track. Better report a false positive than miss a
2241+
time-dependence.
2242+
22392243
:param expr:
22402244
The expression.
22412245
22422246
:returns:
22432247
Whether the expression is time-dependent.
22442248
"""
2245-
# `expr.free_symbols` will be different to `self._states.keys()`, so
2246-
# it's easier to compare as `str`.
2247-
expr_syms = {str(sym) for sym in expr.free_symbols}
2249+
if not (free_syms := expr.free_symbols):
2250+
return False
22482251

2249-
# Check if the time variable is in the expression.
2250-
if amici_time_symbol.name in expr_syms:
2251-
return True
2252+
free_syms -= set(self.sym("p"))
22522253

2253-
# Check if any time-dependent states are in the expression.
2254-
state_syms = [str(sym) for sym in self.states()]
2255-
return any(
2256-
not self.state_is_constant(state_syms.index(state))
2257-
for state in expr_syms.intersection(state_syms)
2258-
)
2254+
if not free_syms:
2255+
return False
2256+
2257+
free_syms -= set(self.sym("k"))
2258+
2259+
# TODO(performance): handle static expressions,
2260+
# handle constant state variables,
2261+
# combine with other checks for time-dependence
2262+
# after https://github.com/AMICI-dev/AMICI/pull/3031
2263+
return bool(free_syms)
22592264

22602265
def _get_unique_root(
22612266
self,

0 commit comments

Comments
 (0)