From a88c056e0be9b02ec4c71a79cc183ff8aa92c662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Fr=C3=B6hlich?= Date: Fri, 31 Jan 2025 18:18:20 +0000 Subject: [PATCH 1/5] fix #2642 --- python/sdist/amici/sbml_import.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/python/sdist/amici/sbml_import.py b/python/sdist/amici/sbml_import.py index 557ad02d0f..1ae00d1486 100644 --- a/python/sdist/amici/sbml_import.py +++ b/python/sdist/amici/sbml_import.py @@ -2292,9 +2292,16 @@ def _make_initial( sym_math, rateof_to_dummy = _rateof_to_dummy(sym_math) - for species_id, species in self.symbols[SymbolId.SPECIES].items(): - if "init" in species: - sym_math = smart_subs(sym_math, species_id, species["init"]) + for var in sym_math.free_symbols: + # already recursive since _get_element_initial_assignment calls _make_initial + ia = self._get_element_initial_assignment(str(var)) + if ia is not None: + sym_math = sym_math.subs(var, ia) + + elif (species := self.sbml.getSpecies(str(var))) is not None: + # recursive! + init = self._make_initial(get_species_initial(species)) + sym_math = sym_math.subs(var, init) sym_math = smart_subs(sym_math, sbml_time_symbol, sp.Float(0)) From c0c0cdf6f54b8d39e39cd6a0541aecc67de933d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Fr=C3=B6hlich?= Date: Fri, 31 Jan 2025 21:14:20 +0000 Subject: [PATCH 2/5] fixup --- python/sdist/amici/sbml_import.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/python/sdist/amici/sbml_import.py b/python/sdist/amici/sbml_import.py index 1ae00d1486..796af2507b 100644 --- a/python/sdist/amici/sbml_import.py +++ b/python/sdist/amici/sbml_import.py @@ -2292,16 +2292,25 @@ def _make_initial( sym_math, rateof_to_dummy = _rateof_to_dummy(sym_math) + # we can't rely on anything else being properly initialized at this point, so we need to + # compute all initial values from scratch, recursively for var in sym_math.free_symbols: + element_id = str(var) # already recursive since _get_element_initial_assignment calls _make_initial - ia = self._get_element_initial_assignment(str(var)) - if ia is not None: + if ( + ia := self._get_element_initial_assignment(element_id) + ) is not None: sym_math = sym_math.subs(var, ia) - - elif (species := self.sbml.getSpecies(str(var))) is not None: + elif (species := self.sbml.getSpecies(element_id)) is not None: # recursive! init = self._make_initial(get_species_initial(species)) sym_math = sym_math.subs(var, init) + elif ( + element := self.sbml.getElementBySId(element_id) + ) and self.is_rate_rule_target(element): + # no need to recurse here, as value is numeric + init = sp.Float(element.getValue()) + sym_math = sym_math.subs(var, init) sym_math = smart_subs(sym_math, sbml_time_symbol, sp.Float(0)) From 3ff2c9dcf7380c8364b6e41ed21b8cd69a592204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Fr=C3=B6hlich?= Date: Fri, 31 Jan 2025 22:12:32 +0000 Subject: [PATCH 3/5] fixup --- python/sdist/amici/sbml_import.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/sdist/amici/sbml_import.py b/python/sdist/amici/sbml_import.py index 796af2507b..29a9608c4b 100644 --- a/python/sdist/amici/sbml_import.py +++ b/python/sdist/amici/sbml_import.py @@ -2305,6 +2305,10 @@ def _make_initial( # recursive! init = self._make_initial(get_species_initial(species)) sym_math = sym_math.subs(var, init) + elif var in self.symbols[SymbolId.SPECIES]: + sym_math = sym_math.subs( + var, self.symbols[SymbolId.SPECIES][var]["init"] + ) elif ( element := self.sbml.getElementBySId(element_id) ) and self.is_rate_rule_target(element): From f72a69479221ab390c9cc4e564b775df733c0998 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Fr=C3=B6hlich?= Date: Wed, 5 Feb 2025 11:46:46 +0000 Subject: [PATCH 4/5] add issue as regression test --- python/tests/sbml_models/regression_2642.xml | 130 +++++++++++++++++++ python/tests/test_sbml_import.py | 23 ++++ 2 files changed, 153 insertions(+) create mode 100644 python/tests/sbml_models/regression_2642.xml diff --git a/python/tests/sbml_models/regression_2642.xml b/python/tests/sbml_models/regression_2642.xml new file mode 100644 index 0000000000..63dd9602d4 --- /dev/null +++ b/python/tests/sbml_models/regression_2642.xml @@ -0,0 +1,130 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + MR_i_t0 + + + + + MR_m_t0 + + + + + + + 0.00585177319092733 + 0.00606 + + + + + + + + + + + + recycling + MR_i + + synthesis_MR + + MR_m + + + + + + + + + + + + synthesis_MR + + + + + + + + + + + + + + + recycling + MR_i + + + + + + + + + + + + + + + + binding + MR_m + + + + + + + diff --git a/python/tests/test_sbml_import.py b/python/tests/test_sbml_import.py index 87b18da92d..c50f7305d3 100644 --- a/python/tests/test_sbml_import.py +++ b/python/tests/test_sbml_import.py @@ -852,3 +852,26 @@ def test_import_same_model_name(): assert model_module_1c.get_model().getParameters()[0] == 1.0 assert model_module_1c.get_model().module is model_module_1c + + +@skip_on_valgrind +def test_regression_2642(): + sbml_file = Path(".") / "sbml_models" / "regression_2642.xml" + sbml_importer = amici.SbmlImporter(sbml_file) + model_name = "regression_2642" + with TemporaryDirectory(prefix="regression_2642") as outdir: + sbml_importer.sbml2amici( + model_name=model_name, + output_dir=outdir, + ) + module = amici.import_model_module( + module_name=model_name, module_path=outdir + ) + model = module.getModel() + solver = model.getSolver() + model.setTimepoints(np.linspace(0, 1, 3)) + r = amici.runAmiciSimulation(model, solver) + assert ( + len(np.unique(r.w[:, model.getExpressionIds().index("binding")])) + == 1 + ) From a0e31135fc572920157a5e56f14b2d7273342b11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Fr=C3=B6hlich?= Date: Wed, 5 Feb 2025 14:41:42 +0000 Subject: [PATCH 5/5] fix path --- python/tests/test_sbml_import.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/tests/test_sbml_import.py b/python/tests/test_sbml_import.py index c50f7305d3..9047b76b4a 100644 --- a/python/tests/test_sbml_import.py +++ b/python/tests/test_sbml_import.py @@ -856,7 +856,7 @@ def test_import_same_model_name(): @skip_on_valgrind def test_regression_2642(): - sbml_file = Path(".") / "sbml_models" / "regression_2642.xml" + sbml_file = Path(__file__).parent / "sbml_models" / "regression_2642.xml" sbml_importer = amici.SbmlImporter(sbml_file) model_name = "regression_2642" with TemporaryDirectory(prefix="regression_2642") as outdir: