From f50214eb32f3624a14a137cc9d94c74ea18f06df Mon Sep 17 00:00:00 2001 From: Torkel Loman Date: Sun, 26 Oct 2025 21:33:30 +0000 Subject: [PATCH 1/4] add manually throw error for wrong input --- src/systems/callbacks.jl | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/systems/callbacks.jl b/src/systems/callbacks.jl index c94166103b..a12d94818b 100644 --- a/src/systems/callbacks.jl +++ b/src/systems/callbacks.jl @@ -11,7 +11,7 @@ struct SymbolicAffect end function SymbolicAffect(affect::Vector{Equation}; alg_eqs = Equation[], - discrete_parameters = Any[], kwargs...) + discrete_parameters = infer_discrete_parameters(affect), kwargs...) if !(discrete_parameters isa AbstractVector) discrete_parameters = Any[discrete_parameters] elseif !(discrete_parameters isa Vector{Any}) @@ -31,6 +31,38 @@ function Symbolics.fast_substitute(aff::SymbolicAffect, rules) map(substituter, aff.discrete_parameters)) end +# The discrete parameters (i.e. parameters that are updated in an event) can be inferred as +# those that occur in an affect equation *outside* of a `Pre(...)` operator. +function infer_discrete_parameters(affects) + discrete_parameters = Set() + for affect in affects + if affect isa Equation + infer_discrete_parameters!(discrete_parameters, affect.lhs) + infer_discrete_parameters!(discrete_parameters, affect.rhs) + elseif affect isa NamedTuple + haskey(affect, :modified) && union!(discrete_parameters, affect.modified) + end + end + return collect(discrete_parameters) +end + +# Find all `expr`'s parameters that occur *outside* of a Pre(...) statement. Add these to `discrete_parameters`. +function infer_discrete_parameters!(discrete_parameters, expr) + expr_pre_removed = Symbolics.replacenode(expr, precall_to_1) + dynamic_symvars = Symbolics.get_variables(expr_pre_removed) + # Change this coming line to a Symbolic append type of thing. + union!(discrete_parameters, filter(ModelingToolkit.isparameter, dynamic_symvars)) +end + +# When updating vector variables, the affect side can be a vector. +function infer_discrete_parameters!(discrete_parameters, expr_vec::Vector) + foreach(expr -> infer_discrete_parameters!(discrete_parameters, expr), expr_vec) +end + +# Functions for replacing a Pre-call with a `1.0` (removing its content from an expression). +is_precall(expr) = iscall(expr) ? operation(expr) isa Pre : false +precall_to_1(expr) = (is_precall(expr) ? 1.0 : expr) + struct AffectSystem """The internal implicit discrete system whose equations are solved to obtain values after the affect.""" system::AbstractSystem @@ -438,8 +470,10 @@ function SymbolicDiscreteCallback( condition::Union{Symbolic{Bool}, Number, Vector{<:Number}}, affect = nothing; initialize = nothing, finalize = nothing, reinitializealg = nothing, kwargs...) - c = is_timed_condition(condition) ? condition : value(scalarize(condition)) + # Manual error check (to prevent events like `[X < 5.0] => [X ~ Pre(X) + 10.0]` from being created). + (condition isa Vector) && (eltype(condition) <: Num) && error("Vectors of symbolic conditions are not allowed for `SymbolicDiscreteCallback`.") + c = is_timed_condition(condition) ? condition : value(scalarize(condition)) if isnothing(reinitializealg) if any(a -> a isa ImperativeAffect, [affect, initialize, finalize]) From 66f1d3178d8551f5dd2fbdf58e396b5c29e63e49 Mon Sep 17 00:00:00 2001 From: Torkel Loman Date: Sun, 26 Oct 2025 21:38:07 +0000 Subject: [PATCH 2/4] add test --- test/symbolic_events.jl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/symbolic_events.jl b/test/symbolic_events.jl index 5f37e524e1..2b33a46f84 100644 --- a/test/symbolic_events.jl +++ b/test/symbolic_events.jl @@ -1440,3 +1440,11 @@ end @mtkcompile sys = MWE() @test_nowarn ODEProblem(sys, [], (0.0, 1.0)) end + +@testset "Test erroneously created events yields errors" begin + @parameters p(t) d + @variables X(t) + @test_throws "Vectors of symbolic conditions are not allowed" SymbolicDiscreteCallback([X < 5.0] => [X ~ Pre(X) + 10.0]) + @test_throws "Vectors of symbolic conditions are not allowed" SymbolicDiscreteCallback([X < 5.0, X > 10.0] => [X ~ Pre(X) + 10.0]) + @test_throws "MethodError: no method" SymbolicContinuousCallback((X < 5.0) => [X ~ Pre(X) + 10.0]) +end \ No newline at end of file From dcec735a2fd50fa251401f6975c2fb789352228c Mon Sep 17 00:00:00 2001 From: Torkel Loman Date: Sun, 26 Oct 2025 21:39:03 +0000 Subject: [PATCH 3/4] formatting --- src/systems/callbacks.jl | 3 ++- test/symbolic_events.jl | 15 +++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/systems/callbacks.jl b/src/systems/callbacks.jl index a12d94818b..4a3ca2aa52 100644 --- a/src/systems/callbacks.jl +++ b/src/systems/callbacks.jl @@ -471,7 +471,8 @@ function SymbolicDiscreteCallback( initialize = nothing, finalize = nothing, reinitializealg = nothing, kwargs...) # Manual error check (to prevent events like `[X < 5.0] => [X ~ Pre(X) + 10.0]` from being created). - (condition isa Vector) && (eltype(condition) <: Num) && error("Vectors of symbolic conditions are not allowed for `SymbolicDiscreteCallback`.") + (condition isa Vector) && (eltype(condition) <: Num) && + error("Vectors of symbolic conditions are not allowed for `SymbolicDiscreteCallback`.") c = is_timed_condition(condition) ? condition : value(scalarize(condition)) if isnothing(reinitializealg) diff --git a/test/symbolic_events.jl b/test/symbolic_events.jl index 2b33a46f84..acf21bc361 100644 --- a/test/symbolic_events.jl +++ b/test/symbolic_events.jl @@ -1444,7 +1444,14 @@ end @testset "Test erroneously created events yields errors" begin @parameters p(t) d @variables X(t) - @test_throws "Vectors of symbolic conditions are not allowed" SymbolicDiscreteCallback([X < 5.0] => [X ~ Pre(X) + 10.0]) - @test_throws "Vectors of symbolic conditions are not allowed" SymbolicDiscreteCallback([X < 5.0, X > 10.0] => [X ~ Pre(X) + 10.0]) - @test_throws "MethodError: no method" SymbolicContinuousCallback((X < 5.0) => [X ~ Pre(X) + 10.0]) -end \ No newline at end of file + @test_throws "Vectors of symbolic conditions are not allowed" SymbolicDiscreteCallback([X < + 5.0] => [X ~ + Pre(X) + + 10.0]) + @test_throws "Vectors of symbolic conditions are not allowed" SymbolicDiscreteCallback([ + X < 5.0, X > 10.0] => [X ~ Pre(X) + 10.0]) + @test_throws "MethodError: no method" SymbolicContinuousCallback((X < + 5.0) => [X ~ + Pre(X) + + 10.0]) +end From 7bfdf3b1a54da4d10599632c84a78c29be01dc2b Mon Sep 17 00:00:00 2001 From: Torkel Loman Date: Mon, 27 Oct 2025 08:44:57 +0000 Subject: [PATCH 4/4] remove wrong code --- src/systems/callbacks.jl | 34 +--------------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/src/systems/callbacks.jl b/src/systems/callbacks.jl index 4a3ca2aa52..348b7b09ec 100644 --- a/src/systems/callbacks.jl +++ b/src/systems/callbacks.jl @@ -11,7 +11,7 @@ struct SymbolicAffect end function SymbolicAffect(affect::Vector{Equation}; alg_eqs = Equation[], - discrete_parameters = infer_discrete_parameters(affect), kwargs...) + discrete_parameters = Any[], kwargs...) if !(discrete_parameters isa AbstractVector) discrete_parameters = Any[discrete_parameters] elseif !(discrete_parameters isa Vector{Any}) @@ -31,38 +31,6 @@ function Symbolics.fast_substitute(aff::SymbolicAffect, rules) map(substituter, aff.discrete_parameters)) end -# The discrete parameters (i.e. parameters that are updated in an event) can be inferred as -# those that occur in an affect equation *outside* of a `Pre(...)` operator. -function infer_discrete_parameters(affects) - discrete_parameters = Set() - for affect in affects - if affect isa Equation - infer_discrete_parameters!(discrete_parameters, affect.lhs) - infer_discrete_parameters!(discrete_parameters, affect.rhs) - elseif affect isa NamedTuple - haskey(affect, :modified) && union!(discrete_parameters, affect.modified) - end - end - return collect(discrete_parameters) -end - -# Find all `expr`'s parameters that occur *outside* of a Pre(...) statement. Add these to `discrete_parameters`. -function infer_discrete_parameters!(discrete_parameters, expr) - expr_pre_removed = Symbolics.replacenode(expr, precall_to_1) - dynamic_symvars = Symbolics.get_variables(expr_pre_removed) - # Change this coming line to a Symbolic append type of thing. - union!(discrete_parameters, filter(ModelingToolkit.isparameter, dynamic_symvars)) -end - -# When updating vector variables, the affect side can be a vector. -function infer_discrete_parameters!(discrete_parameters, expr_vec::Vector) - foreach(expr -> infer_discrete_parameters!(discrete_parameters, expr), expr_vec) -end - -# Functions for replacing a Pre-call with a `1.0` (removing its content from an expression). -is_precall(expr) = iscall(expr) ? operation(expr) isa Pre : false -precall_to_1(expr) = (is_precall(expr) ? 1.0 : expr) - struct AffectSystem """The internal implicit discrete system whose equations are solved to obtain values after the affect.""" system::AbstractSystem