From daa504b9ce57e566a93249ffcf6db30745cee0f2 Mon Sep 17 00:00:00 2001 From: Christopher Rackauckas Date: Mon, 9 Sep 2024 04:42:18 +0200 Subject: [PATCH 1/2] don't automatically complete systems on read Otherwise they can never be modified! This is making an assumption about future usage that isn't always satisfied. Indeed, many of the tests immediately modify the system afterwards, meaning the complete designation wasn't correct. --- docs/src/index.md | 2 +- src/systems.jl | 4 +--- test/rules.jl | 4 ++-- test/wuschel.jl | 4 ++-- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/docs/src/index.md b/docs/src/index.md index 2ac4608..26f25bc 100644 --- a/docs/src/index.md +++ b/docs/src/index.md @@ -45,7 +45,7 @@ mdl = readSBML("my_model.xml", doc -> begin set_level_and_version(3, 2)(doc) convert_promotelocals_expandfuns(doc) end) -rs = ReactionSystem(mdl) # If you want to create a reaction system +rs = complete(ReactionSystem(mdl)) # If you want to create a reaction system odesys = convert(ODESystem, rs) # Alternatively: ODESystem(mdl) tspan = (0.0, 1.0) diff --git a/src/systems.jl b/src/systems.jl index ffc6e2f..2f34fb8 100644 --- a/src/systems.jl +++ b/src/systems.jl @@ -41,7 +41,6 @@ function SBML.readSBML(sbmlfile::String, ::ODESystemImporter; include_zero_odes::Bool = true, kwargs...) # Returns an MTK.ODESystem odesys = convert(ODESystem, readSBML(sbmlfile, ReactionSystemImporter(), kwargs...), include_zero_odes = include_zero_odes) - complete(odesys) end """ @@ -96,7 +95,7 @@ function Catalyst.ReactionSystem(model::SBML.Model; kwargs...) # Todo: requires defaults = defs, name = gensym(:SBML), continuous_events = get_events(model), combinatoric_ratelaws = false, kwargs...) - return complete(rs) # Todo: maybe add a `complete=True` kwarg + return rs end """ @@ -110,7 +109,6 @@ function ModelingToolkit.ODESystem(model::SBML.Model; include_zero_odes::Bool = kwargs...) rs = ReactionSystem(model; kwargs...) odesys = convert(ODESystem, rs; include_zero_odes = include_zero_odes) - complete(odesys) end function get_mappings(model::SBML.Model) diff --git a/test/rules.jl b/test/rules.jl index 52e51e0..5cbc3a0 100644 --- a/test/rules.jl +++ b/test/rules.jl @@ -61,14 +61,14 @@ vc = SBMLToolkit.get_volume_correction(m, "S1") # tests that non-constant parameters become variables sbml, _, _ = SBMLToolkitTestSuite.read_case("00033") -m = readmodel(sbml) +m = complete(readmodel(sbml)) @named sys = ODESystem(m) @species k1(IV) @test isequal(k1, unknowns(sys)[end]) # tests that non-constant compartments become variables sbml, _, _ = SBMLToolkitTestSuite.read_case("00051") # hOSU="true" species -m = readmodel(sbml) +m = complete(readmodel(sbml)) @named sys = ODESystem(m) @species C(IV) @test isequal(C, unknowns(sys)[end]) diff --git a/test/wuschel.jl b/test/wuschel.jl index b09a11d..961cff3 100644 --- a/test/wuschel.jl +++ b/test/wuschel.jl @@ -14,6 +14,6 @@ end) sys = ODESystem(m) @test length(equations(sys)) == 1012 @test length(unknowns(sys)) == 1012 -#ssys = structural_simplify(sys) # Todo: Figure out why this complains about ExtraVariablesSystemException -prob = ODEProblem(sys, [], (0, 10.0)) +ssys = structural_simplify(sys) # Todo: Figure out why this complains about ExtraVariablesSystemException +prob = ODEProblem(ssys, [], (0, 10.0)) solve(prob, Tsit5(), save_everystep = false) From 491ed96e0f9dc412bee837aae689cb71c24a17cb Mon Sep 17 00:00:00 2001 From: Christopher Rackauckas Date: Mon, 9 Sep 2024 05:29:50 +0200 Subject: [PATCH 2/2] more --- test/rules.jl | 10 +++++----- test/wuschel.jl | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/rules.jl b/test/rules.jl index 5cbc3a0..196102a 100644 --- a/test/rules.jl +++ b/test/rules.jl @@ -16,26 +16,26 @@ end # Test get_rules sbml, _, _ = SBMLToolkitTestSuite.read_case("00029") # assignmentRule -m = readmodel(sbml) +m = complete(readmodel(sbml)) a, o, r = SBMLToolkit.get_rules(m) o_true = [S1 ~ 7 * compartment] @test isequal(o, o_true) sbml, _, _ = SBMLToolkitTestSuite.read_case("00031") # rateRule -m = readmodel(sbml) +m = complete(readmodel(sbml)) a, o, r = SBMLToolkit.get_rules(m) r_true = [default_time_deriv()(S1) ~ 7 * compartment] @test isequal(r, r_true) sbml, _, _ = SBMLToolkitTestSuite.read_case("00039") # algebraicRule -m = readmodel(sbml) +m = complete(readmodel(sbml)) a, o, r = SBMLToolkit.get_rules(m) a_true = [0 ~ S1 + S2 - k1] @test isequal(a, a_true) # Test get_var_and_assignment sbml, _, _ = SBMLToolkitTestSuite.read_case("00031") -m = readmodel(sbml) +m = complete(readmodel(sbml)) var, assignment = SBMLToolkit.get_var_and_assignment(m, m.rules[1]) var_true = S1 assignment_true = 7 * compartment @@ -55,7 +55,7 @@ vc = SBMLToolkit.get_volume_correction(m, "S1") @test isequal(vc, "compartment") sbml, _, _ = SBMLToolkitTestSuite.read_case("00060") # hOSU="true" species -m = readmodel(sbml) +m = complete(readmodel(sbml)) vc = SBMLToolkit.get_volume_correction(m, "S1") @test isnothing(vc) diff --git a/test/wuschel.jl b/test/wuschel.jl index 961cff3..3cc3ca1 100644 --- a/test/wuschel.jl +++ b/test/wuschel.jl @@ -11,7 +11,7 @@ m = readSBMLFromString(sbml, doc -> begin # set_level_and_version(3, 2)(doc) # fails on wuschel convert_promotelocals_expandfuns(doc) end) -sys = ODESystem(m) +sys = ODESystem(complete(m)) @test length(equations(sys)) == 1012 @test length(unknowns(sys)) == 1012 ssys = structural_simplify(sys) # Todo: Figure out why this complains about ExtraVariablesSystemException