Skip to content

Commit 488f022

Browse files
committed
Restore ordering of w
With #3036, the ordering of symbols and expressions has changed. This is not a problem per se, but would require recreating the test oracles for the models in `models/`. Therefore, don't do any unnecessary reordering.
1 parent baed9cd commit 488f022

File tree

2 files changed

+35
-18
lines changed

2 files changed

+35
-18
lines changed

python/sdist/amici/de_model.py

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2583,27 +2583,15 @@ def has_event_assignments(self) -> bool:
25832583
"""
25842584
return any(event.updates_state for event in self._events)
25852585

2586-
def toposort_expressions(self) -> dict[sp.Symbol, sp.Expr]:
2586+
def toposort_expressions(
2587+
self, reorder: bool = True
2588+
) -> dict[sp.Symbol, sp.Expr]:
25872589
"""
25882590
Sort expressions in topological order.
25892591
25902592
:return:
25912593
dict of expression symbols to expressions in topological order
25922594
"""
2593-
# ensure no symbols or equations that depend on `w` have been generated
2594-
# yet, otherwise the re-ordering might break dependencies
2595-
if (
2596-
generated := set(self._syms)
2597-
| set(self._eqs)
2598-
| set(self._sparsesyms)
2599-
| set(self._sparseeqs)
2600-
) - {"w", "p", "k", "x", "x_rdata"}:
2601-
raise AssertionError(
2602-
"This function must be called before computing any "
2603-
"derivatives. The following symbols/equations are already "
2604-
f"generated: {generated}"
2605-
)
2606-
26072595
# NOTE: elsewhere, conservations law expressions are expected to
26082596
# occur before any other w expressions, so we must maintain their
26092597
# position.
@@ -2622,6 +2610,23 @@ def toposort_expressions(self) -> dict[sp.Symbol, sp.Expr]:
26222610
for e in self.expressions()[: self.num_cons_law()]
26232611
} | w_toposorted
26242612

2613+
if not reorder:
2614+
return w_toposorted
2615+
2616+
# ensure no symbols or equations that depend on `w` have been generated
2617+
# yet, otherwise the re-ordering might break dependencies
2618+
if (
2619+
generated := set(self._syms)
2620+
| set(self._eqs)
2621+
| set(self._sparsesyms)
2622+
| set(self._sparseeqs)
2623+
) - {"w", "p", "k", "x", "x_rdata"}:
2624+
raise AssertionError(
2625+
"This function must be called before computing any "
2626+
"derivatives. The following symbols/equations are already "
2627+
f"generated: {generated}"
2628+
)
2629+
26252630
old_syms = tuple(e.get_sym() for e in self.expressions())
26262631
topo_expr_syms = tuple(w_toposorted)
26272632
new_order = [old_syms.index(s) for s in topo_expr_syms]

python/sdist/amici/importers/sbml/__init__.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,10 @@
6464
toposort_symbols,
6565
)
6666
from amici.logging import get_logger, log_execution_time, set_log_level
67-
from amici.sympy_utils import smart_is_zero_matrix, smart_multiply
67+
from amici.sympy_utils import (
68+
smart_is_zero_matrix,
69+
smart_multiply,
70+
)
6871

6972
SymbolicFormula = dict[sp.Symbol, sp.Expr]
7073

@@ -3089,11 +3092,20 @@ def do_subs(expr, rate_ofs) -> sp.Expr:
30893092

30903093
# replace rateOf-instances in expressions which we will need for
30913094
# substitutions later
3095+
expressions_changed = False
30923096
for expr in de_model.expressions():
30933097
if rate_ofs := expr.get_val().find(rate_of_func):
30943098
expr.set_val(do_subs(expr.get_val(), rate_ofs))
3095-
3096-
w_toposorted = de_model.toposort_expressions()
3099+
expressions_changed = True
3100+
3101+
# get toposorted `w`, but only changed the order of expressions in the
3102+
# model if any expression changed.
3103+
# (in principle, we could always reorder the expressions, but this
3104+
# will require regenerating all test oracle for
3105+
# `test_pregenerated_models.py`)
3106+
w_toposorted = de_model.toposort_expressions(
3107+
reorder=expressions_changed
3108+
)
30973109

30983110
# replace rateOf-instances in x0
30993111
# indices of state variables whose x0 was modified

0 commit comments

Comments
 (0)