..

Author: dweindl

include/amici/model_dae.h

@@ -446,7 +446,7 @@ class Model_DAE : public Model {
      * @param x Vector with the states
      * @param p parameter vector
      * @param k constants vector
-     * @param h heavyside vector
+     * @param h Heaviside vector
      * @param dx Vector with the derivative states
      * @param w vector with helper variables
      */

python/sdist/amici/de_model.py

@@ -652,6 +652,9 @@ def num_events_solver(self) -> int:
         :return:
             number of event symbols (length of the root vector in AMICI)
         """
+        # TODO(performance): we could include constant `x` here as well
+        #   (dx/dt = 0 AND not target of event assignments)
+        #   this will require passing `x` to `fexplicit_roots`
         static_syms = self._static_symbols(["k", "p", "w"])
         return sum(
             not event.has_explicit_trigger_times(static_syms)
@@ -1138,6 +1141,7 @@ def generate_basic_variables(self) -> None:
         Generates the symbolic identifiers for all variables in
         ``DEModel._variable_prototype``
         """
+        self.parse_events()
         self._reorder_events()
 
         for var in self._variable_prototype:
@@ -1164,11 +1168,6 @@ def parse_events(self) -> None:
         for expr in self._expressions:
             expr.set_val(self._process_heavisides(expr.get_val(), roots))
 
-        # remove all possible Heavisides from roots, which may arise from
-        # the substitution of `'w'` in `_collect_heaviside_roots`
-        for root in roots:
-            root.set_val(self._process_heavisides(root.get_val(), roots))
-
         # Now add the found roots to the model components
         for root in roots:
             # skip roots of SBML events, as these have already been added

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

@@ -398,7 +398,6 @@ def ode_model_from_pysb_importer(
 
     _process_stoichiometric_matrix(model, ode, fixed_parameters)
 
-    ode.parse_events()
     ode.generate_basic_variables()
 
     return ode

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

@@ -700,9 +700,7 @@ def _build_ode_model(
         if hybridization:
             ode_model._process_hybridization(hybridization)
 
-        ode_model.parse_events()
         # substitute SBML-rateOf constructs
-        #  must be done after parse_events, but before generate_basic_variables
         self._process_sbml_rate_of(ode_model)
 
         # fill in 'self._sym' based on prototypes and components in ode_model

python/tests/test_pysb.py

@@ -328,7 +328,7 @@ def test_names_and_ids(pysb_example_presimulation_module):
 
 
 @skip_on_valgrind
-def test_heavyside_and_special_symbols():
+def test_heaviside_and_special_symbols():
     pysb.SelfExporter.cleanup()  # reset pysb
     pysb.SelfExporter.do_export = True
 

src/model_dae.cpp

@@ -95,7 +95,8 @@ void Model_DAE::froot(
 ) {
     std::ranges::fill(root, 0.0);
     auto const x_pos = compute_x_pos(x);
-    // TODO too costly? only dynamic expressions?
+    // TODO(performance) only dynamic expressions? consider storing a flag
+    //  for whether froot actually depends on `w`.
     fw(t, N_VGetArrayPointerConst(x_pos));
     froot(
         root.data(), t, N_VGetArrayPointerConst(x_pos),

src/model_ode.cpp

@@ -84,7 +84,8 @@ void Model_ODE::froot(
     realtype const t, const_N_Vector x, gsl::span<realtype> root
 ) {
     auto const x_pos = compute_x_pos(x);
-    // TODO too costly? only dynamic expressions?
+    // TODO(performance) only dynamic expressions? consider storing a flag
+    //  for whether froot actually depends on `w`.
     fw(t, N_VGetArrayPointerConst(x_pos));
     std::ranges::fill(root, 0.0);
     froot(