From 890c83949571c2446782230168539e6d9c5eca6c Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 17 Nov 2025 21:40:35 +0100 Subject: [PATCH] Restore ordering of `w` With #3036, the ordering of `w` 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. --- python/sdist/amici/de_model.py | 39 ++++++++++++------- python/sdist/amici/importers/sbml/__init__.py | 13 ++++++- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/python/sdist/amici/de_model.py b/python/sdist/amici/de_model.py index c2821237b9..3807f96552 100644 --- a/python/sdist/amici/de_model.py +++ b/python/sdist/amici/de_model.py @@ -2588,27 +2588,19 @@ def has_event_assignments(self) -> bool: """ return any(event.updates_state for event in self._events) - def toposort_expressions(self) -> dict[sp.Symbol, sp.Expr]: + def toposort_expressions( + self, reorder: bool = True + ) -> dict[sp.Symbol, sp.Expr]: """ Sort expressions in topological order. + :param reorder: + Whether to reorder the internal expression list (``True``) or + just return the toposorted expressions (``False``). + :return: dict of expression symbols to expressions in topological order """ - # ensure no symbols or equations that depend on `w` have been generated - # yet, otherwise the re-ordering might break dependencies - if ( - generated := set(self._syms) - | set(self._eqs) - | set(self._sparsesyms) - | set(self._sparseeqs) - ) - {"w", "p", "k", "x", "x_rdata"}: - raise AssertionError( - "This function must be called before computing any " - "derivatives. The following symbols/equations are already " - f"generated: {generated}" - ) - # NOTE: elsewhere, conservations law expressions are expected to # occur before any other w expressions, so we must maintain their # position. @@ -2627,6 +2619,23 @@ def toposort_expressions(self) -> dict[sp.Symbol, sp.Expr]: for e in self.expressions()[: self.num_cons_law()] } | w_toposorted + if not reorder: + return w_toposorted + + # ensure no symbols or equations that depend on `w` have been generated + # yet, otherwise the re-ordering might break dependencies + if ( + generated := set(self._syms) + | set(self._eqs) + | set(self._sparsesyms) + | set(self._sparseeqs) + ) - {"w", "p", "k", "x", "x_rdata"}: + raise AssertionError( + "This function must be called before computing any " + "derivatives. The following symbols/equations are already " + f"generated: {generated}" + ) + old_syms = tuple(e.get_sym() for e in self.expressions()) topo_expr_syms = tuple(w_toposorted) new_order = [old_syms.index(s) for s in topo_expr_syms] diff --git a/python/sdist/amici/importers/sbml/__init__.py b/python/sdist/amici/importers/sbml/__init__.py index 199deefc00..37e2776f8a 100644 --- a/python/sdist/amici/importers/sbml/__init__.py +++ b/python/sdist/amici/importers/sbml/__init__.py @@ -3094,11 +3094,20 @@ def do_subs(expr, rate_ofs) -> sp.Expr: # replace rateOf-instances in expressions which we will need for # substitutions later + expressions_changed = False for expr in de_model.expressions(): if rate_ofs := expr.get_val().find(rate_of_func): expr.set_val(do_subs(expr.get_val(), rate_ofs)) - - w_toposorted = de_model.toposort_expressions() + expressions_changed = True + + # get toposorted `w`, but only changed the order of expressions in the + # model if any expression changed. + # (in principle, we could always reorder the expressions, but this + # will require regenerating all test oracle for + # `test_pregenerated_models.py`) + w_toposorted = de_model.toposort_expressions( + reorder=expressions_changed + ) # replace rateOf-instances in x0 # indices of state variables whose x0 was modified