Merge pull request #3035 from SciML/revert-3027-double_ss_error

Throw an error if structural simplification is applied twice (take 2)

.github/workflows/Tests.yml

@@ -28,8 +28,6 @@ jobs:
         group:
           - InterfaceI
           - InterfaceII
-          - SymbolicIndexingInterface
-          - Extended
           - Extensions
           - Downstream
           - RegressionI

src/structural_transformation/StructuralTransformations.jl

@@ -24,7 +24,7 @@ using ModelingToolkit: ODESystem, AbstractSystem, var_from_nested_derivative, Di
                        invalidate_cache!, Substitutions, get_or_construct_tearing_state,
                        filter_kwargs, lower_varname_with_unit, setio, SparseMatrixCLIL,
                        get_fullvars, has_equations, observed,
-                       Schedule
+                       Schedule, schedule
 
 using ModelingToolkit.BipartiteGraphs
 import .BipartiteGraphs: invview, complete

src/structural_transformation/symbolics_tearing.jl

@@ -125,6 +125,7 @@ function tearing_substitution(sys::AbstractSystem; kwargs...)
     neweqs = full_equations(sys::AbstractSystem; kwargs...)
     @set! sys.eqs = neweqs
     @set! sys.substitutions = nothing
+    @set! sys.schedule = nothing
 end
 
 function tearing_assignments(sys::AbstractSystem)
@@ -615,6 +616,7 @@ function tearing_reassemble(state::TearingState, var_eq_matching,
     if sys isa ODESystem
         @set! sys.schedule = Schedule(var_eq_matching, dummy_sub)
     end
+    sys = schedule(sys)
     @set! state.sys = sys
     @set! sys.tearing_state = state
     return invalidate_cache!(sys)

src/systems/abstractsystem.jl

@@ -849,6 +849,33 @@ iscomplete(sys::AbstractSystem) = isdefined(sys, :complete) && getfield(sys, :co
 """
 $(TYPEDSIGNATURES)
 
+Mark a system as scheduled. It is only intended in compiler internals. A system
+is scheduled after tearing based simplifications where equations are converted
+into assignments.
+"""
+function schedule(sys::AbstractSystem)
+    has_schedule(sys) ? sys : (@set! sys.isscheduled = true)
+end
+
+"""
+$(TYPEDSIGNATURES)
+
+If a system is scheduled, then changing its equations, variables, and
+parameters is no longer legal.
+"""
+function isscheduled(sys::AbstractSystem)
+    if has_schedule(sys)
+        get_schedule(sys) !== nothing
+    elseif has_isscheduled(sys)
+        get_isscheduled(sys)
+    else
+        false
+    end
+end
+
+"""
+$(TYPEDSIGNATURES)
+
 Mark a system as completed. If a system is complete, the system will no longer
 namespace its subsystems or variables, i.e. `isequal(complete(sys).v.i, v.i)`.
 """
@@ -907,7 +934,8 @@ for prop in [:eqs
              :split_idxs
              :parent
              :index_cache
-             :is_scalar_noise]
+             :is_scalar_noise
+             :isscheduled]
     fname_get = Symbol(:get_, prop)
     fname_has = Symbol(:has_, prop)
     @eval begin

src/systems/diffeqs/sdesystem.jl

@@ -133,13 +133,15 @@ struct SDESystem <: AbstractODESystem
     be `true` when `noiseeqs isa Vector`. 
     """
     is_scalar_noise::Bool
+    isscheduled::Bool
 
     function SDESystem(tag, deqs, neqs, iv, dvs, ps, tspan, var_to_name, ctrls, observed,
             tgrad,
             jac,
             ctrl_jac, Wfact, Wfact_t, name, systems, defaults, connector_type,
             cevents, devents, parameter_dependencies, metadata = nothing, gui_metadata = nothing,
-            complete = false, index_cache = nothing, parent = nothing, is_scalar_noise = false;
+            complete = false, index_cache = nothing, parent = nothing, is_scalar_noise = false,
+            isscheduled = false;
             checks::Union{Bool, Int} = true)
         if checks == true || (checks & CheckComponents) > 0
             check_independent_variables([iv])
@@ -162,7 +164,8 @@ struct SDESystem <: AbstractODESystem
         new(tag, deqs, neqs, iv, dvs, ps, tspan, var_to_name, ctrls, observed, tgrad, jac,
             ctrl_jac,
             Wfact, Wfact_t, name, systems, defaults, connector_type, cevents, devents,
-            parameter_dependencies, metadata, gui_metadata, complete, index_cache, parent, is_scalar_noise)
+            parameter_dependencies, metadata, gui_metadata, complete, index_cache, parent, is_scalar_noise,
+            isscheduled)
     end
 end
 

src/systems/discrete_system/discrete_system.jl

@@ -91,14 +91,16 @@ struct DiscreteSystem <: AbstractTimeDependentSystem
     The hierarchical parent system before simplification.
     """
     parent::Any
+    isscheduled::Bool
 
     function DiscreteSystem(tag, discreteEqs, iv, dvs, ps, tspan, var_to_name,
             observed,
             name,
             systems, defaults, preface, connector_type, parameter_dependencies = Equation[],
             metadata = nothing, gui_metadata = nothing,
             tearing_state = nothing, substitutions = nothing,
-            complete = false, index_cache = nothing, parent = nothing;
+            complete = false, index_cache = nothing, parent = nothing,
+            isscheduled = false;
             checks::Union{Bool, Int} = true)
         if checks == true || (checks & CheckComponents) > 0
             check_independent_variables([iv])
@@ -113,7 +115,7 @@ struct DiscreteSystem <: AbstractTimeDependentSystem
             systems,
             defaults,
             preface, connector_type, parameter_dependencies, metadata, gui_metadata,
-            tearing_state, substitutions, complete, index_cache, parent)
+            tearing_state, substitutions, complete, index_cache, parent, isscheduled)
     end
 end
 

src/systems/jumps/jumpsystem.jl

@@ -114,12 +114,13 @@ struct JumpSystem{U <: ArrayPartition} <: AbstractTimeDependentSystem
     Cached data for fast symbolic indexing.
     """
     index_cache::Union{Nothing, IndexCache}
+    isscheduled::Bool
 
     function JumpSystem{U}(tag, ap::U, iv, unknowns, ps, var_to_name, observed, name,
             systems,
             defaults, connector_type, devents, parameter_dependencies,
             metadata = nothing, gui_metadata = nothing,
-            complete = false, index_cache = nothing;
+            complete = false, index_cache = nothing, isscheduled = false;
             checks::Union{Bool, Int} = true) where {U <: ArrayPartition}
         if checks == true || (checks & CheckComponents) > 0
             check_independent_variables([iv])
@@ -132,7 +133,7 @@ struct JumpSystem{U <: ArrayPartition} <: AbstractTimeDependentSystem
         end
         new{U}(tag, ap, iv, unknowns, ps, var_to_name, observed, name, systems, defaults,
             connector_type, devents, parameter_dependencies, metadata, gui_metadata,
-            complete, index_cache)
+            complete, index_cache, isscheduled)
     end
 end
 function JumpSystem(tag, ap, iv, states, ps, var_to_name, args...; kwargs...)

src/systems/nonlinear/nonlinearsystem.jl

@@ -89,21 +89,22 @@ struct NonlinearSystem <: AbstractTimeIndependentSystem
     The hierarchical parent system before simplification.
     """
     parent::Any
+    isscheduled::Bool
 
     function NonlinearSystem(tag, eqs, unknowns, ps, var_to_name, observed, jac, name,
             systems,
             defaults, connector_type, parameter_dependencies = Equation[], metadata = nothing,
             gui_metadata = nothing,
             tearing_state = nothing, substitutions = nothing,
-            complete = false, index_cache = nothing, parent = nothing; checks::Union{
-                Bool, Int} = true)
+            complete = false, index_cache = nothing, parent = nothing,
+            isscheduled = false; checks::Union{Bool, Int} = true)
         if checks == true || (checks & CheckUnits) > 0
             u = __get_unit_type(unknowns, ps)
             check_units(u, eqs)
         end
         new(tag, eqs, unknowns, ps, var_to_name, observed, jac, name, systems, defaults,
             connector_type, parameter_dependencies, metadata, gui_metadata, tearing_state,
-            substitutions, complete, index_cache, parent)
+            substitutions, complete, index_cache, parent, isscheduled)
     end
 end
 

src/systems/optimization/optimizationsystem.jl

@@ -64,10 +64,12 @@ struct OptimizationSystem <: AbstractOptimizationSystem
     The hierarchical parent system before simplification.
     """
     parent::Any
+    isscheduled::Bool
 
     function OptimizationSystem(tag, op, unknowns, ps, var_to_name, observed,
             constraints, name, systems, defaults, metadata = nothing,
-            gui_metadata = nothing, complete = false, index_cache = nothing, parent = nothing;
+            gui_metadata = nothing, complete = false, index_cache = nothing, parent = nothing,
+            isscheduled = false;
             checks::Union{Bool, Int} = true)
         if checks == true || (checks & CheckUnits) > 0
             u = __get_unit_type(unknowns, ps)
@@ -77,7 +79,7 @@ struct OptimizationSystem <: AbstractOptimizationSystem
         end
         new(tag, op, unknowns, ps, var_to_name, observed,
             constraints, name, systems, defaults, metadata, gui_metadata, complete,
-            index_cache, parent)
+            index_cache, parent, isscheduled)
     end
 end
 

src/systems/systems.jl

@@ -29,7 +29,7 @@ function structural_simplify(
         sys::AbstractSystem, io = nothing; simplify = false, split = true,
         allow_symbolic = false, allow_parameter = true, conservative = false, fully_determined = true,
         kwargs...)
-    iscomplete(sys) && throw(RepeatedStructuralSimplificationError())
+    isscheduled(sys) && throw(RepeatedStructuralSimplificationError())
     newsys′ = __structural_simplify(sys, io; simplify,
         allow_symbolic, allow_parameter, conservative, fully_determined,
         kwargs...)

test/components.jl

@@ -44,6 +44,7 @@ completed_rc_model = complete(rc_model)
 @test ModelingToolkit.n_extra_equations(capacitor) == 2
 @test length(equations(structural_simplify(rc_model, allow_parameter = false))) == 2
 sys = structural_simplify(rc_model)
+@test_throws ModelingToolkit.RepeatedStructuralSimplificationError structural_simplify(sys)
 @test length(equations(sys)) == 1
 check_contract(sys)
 @test !isempty(ModelingToolkit.defaults(sys))

test/downstream/linearize.jl

@@ -312,6 +312,7 @@ matrices = linfun([1.0], Dict(p => 3.0), 1.0)
     @parameters p
     eqs = [0 ~ x * log(y) - p]
     @named sys = ODESystem(eqs, t; defaults = [p => 1.0])
+    sys = complete(sys)
     @test_nowarn linearize(
         sys, [x], []; op = Dict(x => 1.0), allow_input_derivatives = true)
 end

test/input_output_handling.jl

@@ -362,12 +362,12 @@ disturbed_input = ins[1]
     dist_integ,
     ins)
 
-getter_sys = complete(augmented_sys)
+augmented_sys = complete(augmented_sys)
 matrices, ssys = linearize(augmented_sys,
     [
-        getter_sys.u,
-        getter_sys.input.u[2],
-        getter_sys.d
+        augmented_sys.u,
+        augmented_sys.input.u[2],
+        augmented_sys.d
     ], outs)
 @test matrices.A ≈ [A [1; 0]; zeros(1, 2) -0.001]
 @test matrices.B == I

test/mtkparameters.jl

@@ -144,8 +144,8 @@ function level1()
     eqs = [D(x) ~ p1 * x - p2 * x * y
            D(y) ~ -p3 * y + p4 * x * y]
 
-    sys = structural_simplify(ODESystem(
-        eqs, t, tspan = (0, 3.0), name = :sys, parameter_dependencies = [y0 => 2p4]))
+    sys = structural_simplify(complete(ODESystem(
+        eqs, t, tspan = (0, 3.0), name = :sys, parameter_dependencies = [y0 => 2p4])))
     prob = ODEProblem{true, SciMLBase.FullSpecialize}(sys)
 end
 
@@ -158,8 +158,8 @@ function level2()
     eqs = [D(x) ~ p1 * x - p23[1] * x * y
            D(y) ~ -p23[2] * y + p4 * x * y]
 
-    sys = structural_simplify(ODESystem(
-        eqs, t, tspan = (0, 3.0), name = :sys, parameter_dependencies = [y0 => 2p4]))
+    sys = structural_simplify(complete(ODESystem(
+        eqs, t, tspan = (0, 3.0), name = :sys, parameter_dependencies = [y0 => 2p4])))
     prob = ODEProblem{true, SciMLBase.FullSpecialize}(sys)
 end
 
@@ -172,8 +172,8 @@ function level3()
     eqs = [D(x) ~ p1 * x - p23[1] * x * y
            D(y) ~ -p23[2] * y + p4 * x * y]
 
-    sys = structural_simplify(ODESystem(
-        eqs, t, tspan = (0, 3.0), name = :sys, parameter_dependencies = [y0 => 2p4]))
+    sys = structural_simplify(complete(ODESystem(
+        eqs, t, tspan = (0, 3.0), name = :sys, parameter_dependencies = [y0 => 2p4])))
     prob = ODEProblem{true, SciMLBase.FullSpecialize}(sys)
 end
 

test/nonlinearsystem.jl

@@ -251,6 +251,7 @@ end
         0 ~ z - cos(x),
         0 ~ x * y]
     @named ns = NonlinearSystem(eqs, [x, y, z], [])
+    ns = complete(ns)
     vs = [unknowns(ns); parameters(ns)]
     ss_mtk = structural_simplify(ns)
     prob = NonlinearProblem(ss_mtk, vs .=> 1.0)

test/reduction.jl

@@ -75,6 +75,11 @@ __x = x
 # Reduced Flattened System
 
 reduced_system = structural_simplify(connected)
+reduced_system2 = structural_simplify(tearing_substitution(structural_simplify(tearing_substitution(structural_simplify(connected)))))
+
+@test isempty(setdiff(unknowns(reduced_system), unknowns(reduced_system2)))
+@test isequal(equations(tearing_substitution(reduced_system)), equations(reduced_system2))
+@test isequal(observed(reduced_system), observed(reduced_system2))
 @test setdiff(unknowns(reduced_system),
     [s
      a

test/runtests.jl

@@ -32,62 +32,57 @@ end
             @safetestset "Dynamic Quantities Test" include("dq_units.jl")
             @safetestset "Unitful Quantities Test" include("units.jl")
             @safetestset "Mass Matrix Test" include("mass_matrix.jl")
+            @safetestset "SteadyStateSystem Test" include("steadystatesystems.jl")
+            @safetestset "SDESystem Test" include("sdesystem.jl")
+            @safetestset "DDESystem Test" include("dde.jl")
+            @safetestset "NonlinearSystem Test" include("nonlinearsystem.jl")
             @safetestset "InitializationSystem Test" include("initializationsystem.jl")
             @safetestset "Guess Propagation" include("guess_propagation.jl")
             @safetestset "Hierarchical Initialization Equations" include("hierarchical_initialization_eqs.jl")
+            @safetestset "PDE Construction Test" include("pde.jl")
+            @safetestset "JumpSystem Test" include("jumpsystem.jl")
+            @safetestset "Constraints Test" include("constraints.jl")
             @safetestset "Reduction Test" include("reduction.jl")
             @safetestset "Split Parameters Test" include("split_parameters.jl")
             @safetestset "StaticArrays Test" include("static_arrays.jl")
             @safetestset "Components Test" include("components.jl")
             @safetestset "Model Parsing Test" include("model_parsing.jl")
+            @safetestset "print_tree" include("print_tree.jl")
             @safetestset "Error Handling" include("error_handling.jl")
             @safetestset "StructuralTransformations" include("structural_transformation/runtests.jl")
             @safetestset "State Selection Test" include("state_selection.jl")
             @safetestset "Symbolic Event Test" include("symbolic_events.jl")
             @safetestset "Stream Connect Test" include("stream_connectors.jl")
             @safetestset "Domain Connect Test" include("domain_connectors.jl")
             @safetestset "Lowering Integration Test" include("lowering_solving.jl")
+            @safetestset "Test Big System Usage" include("bigsystem.jl")
             @safetestset "Dependency Graph Test" include("dep_graphs.jl")
             @safetestset "Function Registration Test" include("function_registration.jl")
             @safetestset "Precompiled Modules Test" include("precompile_test.jl")
+            @safetestset "Variable Utils Test" include("variable_utils.jl")
+            @safetestset "Variable Metadata Test" include("test_variable_metadata.jl")
             @safetestset "DAE Jacobians Test" include("dae_jacobian.jl")
             @safetestset "Jacobian Sparsity" include("jacobiansparsity.jl")
             @safetestset "Modelingtoolkitize Test" include("modelingtoolkitize.jl")
+            @safetestset "OptimizationSystem Test" include("optimizationsystem.jl")
             @safetestset "FuncAffect Test" include("funcaffect.jl")
             @safetestset "Constants Test" include("constants.jl")
             @safetestset "Parameter Dependency Test" include("parameter_dependencies.jl")
             @safetestset "Generate Custom Function Test" include("generate_custom_function.jl")
             @safetestset "Initial Values Test" include("initial_values.jl")
+            @safetestset "Discrete System" include("discrete_system.jl")
             @safetestset "Equation Type Accessors Test" include("equation_type_accessors.jl")
             @safetestset "Equations with complex values" include("complex.jl")
         end
     end
 
-    if GROUP == "All" || GROUP == "InterfaceII"
-        @testset "InterfaceII" begin
-            @safetestset "Variable Utils Test" include("variable_utils.jl")
-            @safetestset "Variable Metadata Test" include("test_variable_metadata.jl")
-            @safetestset "OptimizationSystem Test" include("optimizationsystem.jl")
-            @safetestset "Discrete System" include("discrete_system.jl")
-            @safetestset "SteadyStateSystem Test" include("steadystatesystems.jl")
-            @safetestset "SDESystem Test" include("sdesystem.jl")
-            @safetestset "DDESystem Test" include("dde.jl")
-            @safetestset "NonlinearSystem Test" include("nonlinearsystem.jl")
-            @safetestset "PDE Construction Test" include("pde.jl")
-            @safetestset "JumpSystem Test" include("jumpsystem.jl")
-            @safetestset "print_tree" include("print_tree.jl")
-            @safetestset "Constraints Test" include("constraints.jl")
-        end
-    end
-
-    if GROUP == "All" || GROUP == "SymbolicIndexingInterface"
+    if GROUP == "All" || GROUP == "InterfaceI" || GROUP == "SymbolicIndexingInterface"
         @safetestset "SymbolicIndexingInterface test" include("symbolic_indexing_interface.jl")
         @safetestset "SciML Problem Input Test" include("sciml_problem_inputs.jl")
         @safetestset "MTKParameters Test" include("mtkparameters.jl")
     end
 
-    if GROUP == "All" || GROUP == "Extended"
-        @safetestset "Test Big System Usage" include("bigsystem.jl")
+    if GROUP == "All" || GROUP == "InterfaceII"
         println("C compilation test requires gcc available in the path!")
         @safetestset "C Compilation Test" include("ccompile.jl")
         @testset "Distributed Test" include("distributed.jl")