Skip to content
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

It is possible to designate non-default types for variables, which are then ignored. #2589

Open
TorkelE opened this issue Mar 30, 2024 · 7 comments · May be fixed by #3340
Open

It is possible to designate non-default types for variables, which are then ignored. #2589

TorkelE opened this issue Mar 30, 2024 · 7 comments · May be fixed by #3340
Assignees
Labels
bug Something isn't working

Comments

@TorkelE
Copy link
Member

TorkelE commented Mar 30, 2024

Currently, it is possible to designate a non-default type for variables:

using ModelingToolkit, OrdinaryDiffEq
using ModelingToolkit: t_nounits as t, D_nounits as D

@parameters p d
@variables X(t)::Int64
eq = D(X) ~ p - d*X
@mtkbuild osys = ODESystem([eq], t)

u0 = [X => 1.0]
ps = [p => 1.0, d => 0.2]
oprob = ODEProblem(osys, u0, (0.0,10.0), ps)
oprob[X] isa Int64 # false
oprob[X] isa Float64 # true
sol = solve(oprob, Tsit5())

In this case, Int64 does not make sense for an ODE variable. While it is internally converted to a Float64, there should be an error instead. Here, it is actually possible to set u0 = [X => 1.5], and things proceed as if X(t)::Int64 was not the case.

This is even the case if a type which does make sense in the context of ODEs is give. I.e. here we use a Float32, but internal conversion to Int64 is still a thing.

@parameters p d
@variables X(t)::Float32
eq = D(X) ~ p - d*X
@mtkbuild osys = ODESystem([eq], t)

u0 = [X => 1.0]
ps = [p => 1.0, d => 0.2]
oprob = ODEProblem(osys, u0, (0.0,10.0), ps)
oprob[X] isa Float32 # false
oprob[X] isa Float64 # true
sol = solve(oprob, Tsit5())

While there are cases where integer variables make sense (e.g. Jump simulations), these are already handled internally (i.e. integer initial conditions are interpreted as giving integers throughout a jump simulation).

The easiest would be to throw an error when a user tries

@variables X(t)::Int64

However, if this is not possible (because there are circumstances when one use variables for other stuff), then at least one should be thrown when the ODESystem (or another system) is created.

@TorkelE TorkelE added the bug Something isn't working label Mar 30, 2024
@ChrisRackauckas
Copy link
Member

For state values, yes. We should probably validate those.

@vyudu
Copy link
Contributor

vyudu commented Jan 16, 2025

What should the behavior be here? I assume each type of system should just do its own validation? Should all the following error?

  • Constructing a ODESystem with a @variables x(t)::Int64
  • Constructing a JumpSystem with a @variables x(t)::Float64
  • Constructing an XSystem with a @variables x(t)::String

Is the internal conversion when the type does make sense (as in the Float32 example above?) the right behavior? If so should it give a warning or an error when a Float32 is given?

@ChrisRackauckas
Copy link
Member

For an ODESystem/SDESystem, if it's an Integer it should error.
For any system, if it's not a number in the state then it should error.

Note that integer and non-number are fine for parameters and discrete variables, so this is only for things that are meant to be continuous variables.

@vyudu
Copy link
Contributor

vyudu commented Jan 17, 2025

Got it, what about the Float32 thing?

@ChrisRackauckas
Copy link
Member

Well the other check should be that all of the variable types should be the same, so it should validate that and error otherwise. If you set a specific floating point type? That's really weird since that isn't part of the structure, so I cannot see why we'd want to support that, so I'd also just error if someone chooses concrete float types

@vyudu
Copy link
Contributor

vyudu commented Jan 24, 2025

Should it actually error if all the variable types are not the same? There are some MTK test cases where this is the case, like this one

@testset "Big test case" begin

@ChrisRackauckas
Copy link
Member

In that case c(t) is observed so it's fine. In fact, we need to be able to support some fancy things in nonlinear systems anyways. I think let's do just the first part for now: if it's a differential variable, then it should be a continuous number.

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

Successfully merging a pull request may close this issue.

3 participants