ModelingToolkit.jl icon indicating copy to clipboard operation
ModelingToolkit.jl copied to clipboard

Check that equations contain at least one variable

Open hersle opened this issue 1 year ago • 11 comments

I suggest throwing this nicer error to close #2727

Checklist

  • [x] Appropriate tests were added
  • [x] Any code changes were done in a way that does not break public API
  • [x] All documentation related to code changes were updated
  • [x] The new code follows the contributor guidelines, in particular the SciML Style Guide and COLPRAC.
  • [x] Any new documentation only uses public API

hersle avatar Jul 16 '24 19:07 hersle

Not entirely sure if/how to proceed with this.

  • Catalyst uses an equation like parameter ~ independent variable for a continuous event trigger and calls check_equations() manually,
  • ModelingToolkitStandardLibrary does something I don't understand,
  • SDESystem seems to check_equations() on some things that are just expressions (I assume they really correspond to left/right sides of real equations).

The other failures I think I could fix straight-forwardly.

hersle avatar Jul 17 '24 17:07 hersle

It seems reasonable one might want an event condition to be when time is equal to a parameter. We could flip the order, but given that independent variables are now being treated as parameters you would still have an equation about equality of two parameters.

Catalyst.ReactionSystem is a new system type (like ODESystem) that therefore reuses a lot of the internal checks MTK has for such systems (it can, for example also encapsulate ODEs and algebraic equations).

isaacsas avatar Jul 17 '24 17:07 isaacsas

(Also, I think the same error would arise from a similar continuous event condition, parameter ~ t, being passed to an ODESystem, so this isn't really a Catalyst-specific issue -- MTK just doesn't have a test on such a case apparently.)

isaacsas avatar Jul 17 '24 17:07 isaacsas

@AayushSabharwal is this incompatible with https://github.com/SciML/ModelingToolkit.jl/pull/2747? I guess we could just not apply this to NonlinearSystem?

ChrisRackauckas avatar Jul 20 '24 15:07 ChrisRackauckas

That also struck my mind, so maybe this check is not a great idea to add now.

One could also argue that it should be legal to have parameter-only equations even in the "main equations" of an ODE system. To be forgiving, MTK could detect them and automatically move them to the initialization problem.

hersle avatar Jul 20 '24 15:07 hersle

One of the changes down the line which may happen is that unknowns and parameters may be more fungible by structural simplification. If D(x) ~ 0 then you could move an unknown value to a parameter. One of the things that prevented this in the past is that this can make it so events have issues, since you then need to change the value. But now we also have discrete values, which are functionally stored as parameters but made to be updated and changed during events, and so they are discrete-time state values. In theory then the determination of what is actually unknown could get smarter. I'm not sure if this necessitates a breaking change though, or if this is what would be the core of a v10.

But if that's the case, it really change this PR and the other parameter initialization one to be much better handled by "the normal flow".

Now one other thing to mention, Modelica has a notion of a fixed=false parameter, where they default to fixed=true, and if it's false then they can be determined by initialization. For more on that, see:

https://mbe.modelica.university/behavior/equations/variables/

The fixed attribute changes the way the start attribute is used when the start attribute is used as an initial condition. Normally, the start attribute is considered a “fallback” initial condition and only used if there are insufficient initial conditions explicitly specified in the initial equation sections. However, if the fixed attribute is set to true, then the start attribute is treated as if it was used as an explicit initial equation (i.e., it is no longer used as a fallback, but instead treated as a strict initial condition).

Another, more obscure, use of the fixed attribute is for “computed parameters”. In rare cases where a parameter cannot be initialized explicitly, it is possible to provide a general equation for the parameter in an initial equation section. But in cases where the parameter is initialized in this way, the fixed attribute for the parameter variable must be set to false.

Default: false (except for parameter variables, where it is true by default)

ChrisRackauckas avatar Jul 20 '24 15:07 ChrisRackauckas

@AayushSabharwal is this incompatible with https://github.com/SciML/ModelingToolkit.jl/pull/2747? I guess we could just not apply this to NonlinearSystem?

#2747 is too old to merge, and will basically need to be redone. It shouldn't really factor in to the design decisions we make. Allowing parameters to be unknowns in the initialization is something that can be done regardless of this PR

AayushSabharwal avatar Jul 22 '24 07:07 AayushSabharwal

Rebase this? I think we should go for it.

ChrisRackauckas avatar Jul 22 '24 16:07 ChrisRackauckas

@ChrisRackauckas

One of the changes down the line which may happen is that unknowns and parameters may be more fungible by structural simplification.

I think this would be fantastic and a step in the right direction.

If D(x) ~ 0 then you could move an unknown value to a parameter.

It is very insightful to understand this as a special case of an even more powerful feature: structural simplification replacing equations by their analytical solutions (e.g. #1586). If you had D(x) ~ x, you can also replace it by x ~ A * exp(t), where A must again be a parameter to account for the initial condition, and so on. Maybe this is obvious; I just like to connect the dots 🙂

hersle avatar Aug 06 '24 14:08 hersle

For now this is fine, but one of my goals is to allow specifying parameter_dependencies as parameter-only equations (enforcing that the LHS is a single parameter) so it'll be removed eventually

AayushSabharwal avatar Aug 06 '24 16:08 AayushSabharwal

It is very insightful to understand this as a special case of an even more powerful feature: structural simplification replacing equations by their analytical solutions (e.g. https://github.com/SciML/ModelingToolkit.jl/issues/1586). If you had D(x) ~ x, you can also replace it by x ~ A * exp(t), where A must again be a parameter to account for the initial condition, and so on. Maybe this is obvious; I just like to connect the dots 🙂

Yup, we need to do a few other analytical solutions first, but it has already started

https://github.com/JuliaSymbolics/Symbolics.jl/pull/1192

Once that's in, integrating analytical solutions to rootfinding into the numeric stack is first, and then should do integrals and ODEs.

ChrisRackauckas avatar Aug 07 '24 11:08 ChrisRackauckas