Skip to content

MTK allows you to create faulty events without throwing an error. #2612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TorkelE opened this issue Apr 4, 2024 · 9 comments
Open

MTK allows you to create faulty events without throwing an error. #2612

TorkelE opened this issue Apr 4, 2024 · 9 comments
Assignees
Labels
bug Something isn't working events

Comments

@TorkelE
Copy link
Member

TorkelE commented Apr 4, 2024

I have gone through the events in MTK. There are several cases where MTK allow your to build a system with a misformated event, and an error is not actually thrown until you create a problem or attempts to simulate it. Especially since it is a fit trick how to format the events, the errors should be thrown at the first possible time (when a system is created).

using ModelingToolkit, OrdinaryDiffEq

@parameters p d
@variables t X(t)
eqs = [Differential(t)(X) ~ p - d*X]

# Example 1: Discrete events with dosage given as a Tuple
discrete_events = (2.0, 5.0) => [X ~ X + 10.0]
@mtkbuild osys = ODESystem(eqs, t; discrete_events)
oprob = ODEProblem(osys, [X => 10], (0.0, 10.), [p => 2.0, d => 0.5]) # Error is thrown here.

# Example 2: Discrete events where single expression condition is given in a vector.
discrete_events = [X < 5.0] => [X ~ X + 10.0]
@mtkbuild osys = ODESystem(eqs, t; discrete_events)
oprob = ODEProblem(osys, [X => 10], (0.0, 10.), [p => 2.0, d => 0.5])
sol = solve(oprob, Tsit5()) # Error is thrown here.

# Example 3: Discrete events where multiple expression conditions are given in a vector.
discrete_events = [X < 5.0, X > 10.0] => [X ~ X + 10.0]
@mtkbuild osys = ODESystem(eqs, t; discrete_events)
oprob = ODEProblem(osys, [X => 10], (0.0, 10.), [p => 2.0, d => 0.5]) # Error is thrown here.

# Example 4: Discrete events that affects parameters and variables.
discrete_events = 5.0 => [X ~ X + 10.0, p ~ 2.0]
@mtkbuild osys = ODESystem(eqs, t; discrete_events)
oprob = ODEProblem(osys, [X => 10], (0.0, 10.), [p => 2.0, d => 0.5]) # Error is thrown here.

# Example 5: Discrete events with non-trivial affect right-hand side.
discrete_events = 5.0 => [X + 1.0 ~ X + 10.0]
@mtkbuild osys = ODESystem(eqs, t; discrete_events)
oprob = ODEProblem(osys, [X => 10], (0.0, 10.), [p => 2.0, d => 0.5]) # Error is thrown here.

# Example 6: Continuous events that affects parameters and variables.
continuous_events = [X ~ 5.0] => [X ~ X + 10.0, p ~ 2.0]
@mtkbuild osys = ODESystem(eqs, t; continuous_events)
oprob = ODEProblem(osys, [X => 10], (0.0, 10.), [p => 2.0, d => 0.5]) # Error is thrown here.

# Example 7: Continuous events with non-trivial affect right-hand side.
continuous_events = [X ~ 5.0] => [X + 1.0 ~ X + 10.0]
@mtkbuild osys = ODESystem(eqs, t; continuous_events)
oprob = ODEProblem(osys, [X => 10], (0.0, 10.), [p => 2.0, d => 0.5]) # Error is thrown here.
@TorkelE TorkelE added the bug Something isn't working label Apr 4, 2024
@baggepinnen
Copy link
Contributor

If we used the shift-index notation to describe the affect, we could actually handle cases like

discrete_events = 5.0 => [X + 1.0 ~ X + 10.0]

since it would be well-defined which X to solve for, and easy to solve

discrete_events = 5.0 => [X(k) + 1.0 ~ X(k-1) + 10.0]

for X(k).

@ChrisRackauckas
Copy link
Member

That's a pretty interesting proposal. We'd have to make validate that only a single shift is given but that would make that implicitness become explicit.

@ChrisRackauckas
Copy link
Member

I think the general answer here is actually the use of an implicit discrete system for the callback designation.

@ChrisRackauckas
Copy link
Member

#2077

@vyudu
Copy link
Member

vyudu commented Jan 9, 2025

What's the relationship between this one and #2077 ? I assume this issue need fixing currently (so that all of Torkel's examples throw an error when the ODESystem is declared) and the ImplicitDiscreteSystem thing is separate?

@ChrisRackauckas
Copy link
Member

Well the way to make all symbolic events in a naturally robust way would be to use ImplicitDiscreteSystem. Instead of having the user define a function like v ~ v + 2, we should make that v ~ prev(v) + 2 so that it's clear what's new and what's old. So it should be a discrete system.

But in reality, the issue with events on DAEs is that they have other conditions to satisfy. If you have for example the pendulum, x ~ prev(x) + 0.5 also means you're changing y, because x^2 + y^2 ~ 1. This is not captured in the event definition right now. But for better reinitializations, what you want to do is interpret x ~ prev(x) + 0.5 as instead the system of equations [x ~ prev(x) + 0.5; x^2 + y^2 ~ 1] appending all algebraic equations to the system, and then solving the complete nonlinear system.

If you do this, then it's simply better defined what the syntax means, then a lot of these other cases can actually be supported.

@TorkelE
Copy link
Member Author

TorkelE commented Jan 22, 2025

So this issue is just about a couple of erroneous inputs that do not generate errors on system creation.

The obvious example would be that

discrete_events = "A dIScreTe EvENtzzz"
@mtkbuild osys = ODESystem(eqs, t; discrete_events)

should (and will) generate an error. However, there are a couple of notations that are wrong and will generate errors at some point of the pipeline. However, this is typically in the solve case. Instead, we should add a check in osys = ODESystem(eqs, t; discrete_events) which actually catches these cases and tells them that the error is incorrectly formatted. Since many of these cases looks like something that could be an event for someone who have only skimmed the docs, it is especially good to tell them early so they understand why they get an error.

Then there is events for DAEs, which as Chris mentions might be a bit of a mess.

@ChrisRackauckas
Copy link
Member

But because we already know that we want to make this not be a separate kind of system and instead just an implicit discrete system, what we can do is just finish ImplicitDiscreteSystem, and then this would all just validate in the constructor for that, just like how we validate ODESystems. The issue is that right now the event code is just weird and non-standard, so by making it standard everything else follows. In other words, we just need to delete the current implementation.

@TorkelE
Copy link
Member Author

TorkelE commented Jan 22, 2025

that I won't disagree with

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working events
Projects
None yet
Development

No branches or pull requests

4 participants