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

Cannot create problems with tuple vector variables #3280

Open
TorkelE opened this issue Dec 19, 2024 · 6 comments
Open

Cannot create problems with tuple vector variables #3280

TorkelE opened this issue Dec 19, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@TorkelE
Copy link
Member

TorkelE commented Dec 19, 2024

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

@variables (X(t))[1:2]
@parameters p[1:2]
eqs = [
    D(X[1]) ~ p[1] - X[1],
    D(X[2]) ~ p[2] - X[2]
]
@named model = ODESystem(eqs, t)
model = complete(model)

# Problem inputs
u0_vec = [X => [1.0, 2.0]]
u0_dict = Dict([X => [1.0, 2.0]])
u0_tup = (X => [1.0, 2.0])
p_vec = [p => [4.0, 5.0]]
p_dict = Dict([p => [4.0, 5.0]])
p_tup = (p => [4.0, 5.0])
tspan = (0.0, 10.0)

# Problem creation:
ODEProblem(model, u0_vec, tspan, p_vec)
ODEProblem(model, u0_vec, tspan, p_dict)
ODEProblem(model, u0_vec, tspan, p_tup)
ODEProblem(model, u0_dict, tspan, p_vec)
ODEProblem(model, u0_dict, tspan, p_dict)
ODEProblem(model, u0_dict, tspan, p_tup)
ODEProblem(model, u0_tup, tspan, p_vec) # Error.
ODEProblem(model, u0_tup, tspan, p_dict) # Error.
ODEProblem(model, u0_tup, tspan, p_tup) # Error.

Probably holds for other problem types as well

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

I'm not sure this is supported? Dictionaries and things that transform to dictionaries are supported. Tuples of pairs I guess could work, we'd need to add it to to_dict. What's the use case here?

@TorkelE
Copy link
Member Author

TorkelE commented Dec 20, 2024

I think Tuples have always been supported, right? For parameters I think they are even required for when you have different types in your values. Right now I don't think it is a thing for variables, but seems weird to specifically disallow it here (especially as it might even be useful for hybrid ODE/Jump systems)

@ChrisRackauckas
Copy link
Member

No, the parameters always uses an MTKParameters so it always constructs a different type-stable object and has a representation independent of the user code there. That's a requirement in general in order to support things like non-tunables.

@isaacsas
Copy link
Member

Tuples were supported in MTK as a way to preserve the type of parameters for years. Then a breaking change was made that upconverted everything to floats in a non-breaking MTK8 release (but tuples were still supported as mappings). Catalyst has used tuple mappings forever because we needed mixed parameter types long before MTK9. There doesn’t seem to be any indication that MTK9 dropped tuple mapping support in the NEWS.md?

@ChrisRackauckas
Copy link
Member

It's a part of the change to MTKParameters. But there's no reason to not support it, we might as well just convert it to a vector since that's always faster, and it would be faster for the user to just give a vector but we might as well collect general collections.

@ChrisRackauckas
Copy link
Member

Wait this is for u0 being a tuple? We've never supported that, the differential equation solver needs vector operations in order to work.

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

No branches or pull requests

3 participants