-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
MTK does not allow building initialization systems for un-simplified systems #3330
Comments
That's not how this should be done. Defaults in variable metadata are just a convenience feature. The source of truth for defaults in a completed system is newsys = ODESystem(Equation[], t, [], [k]; defaults = [k => missing], guesses = [k => 2], name = :foo)
model2 = extend(newsys, model) # `newsys` needs to be first |
I initially tried that, but I saw that no initialization problem was created and this is what prompted me to look at metadata and try to force a change there, but that didn't work either. Note that in my example above I updated both the using ModelingToolkit
using ModelingToolkit: D_nounits as D, t_nounits as t
using SymbolicIndexingInterface
using SciMLBase, OrdinaryDiffEqDefault
ps = @parameters k = 2 w = 3
sts = @variables x(t) = 1.5
eqs = [
D(x) ~ -k * x
]
model = structural_simplify(ODESystem(eqs, t, sts, ps; name=:model))
newsys = ODESystem(Equation[], t, [], [k]; defaults=[k => missing], guesses=[k => 2], name=:foo)
model2 = complete(extend(newsys, model)) # `newsys` needs to be first
prob = ODEProblem(model2, [], (0., 1), [k => w])
prob.f.initialization_data # nothing
(u0, p) = setsym_oop(prob, w)(prob, 3.1)
prob_new = remake(prob; u0, p)
init(prob_new).ps[k] # 3.0 |
Initialization not being created is a different issue. That happens because according to MTK |
As a workaround, |
Thanks! Should I close this issue then? |
I think we can keep it up as a tracking issue for allowing initialization without |
I see that on a hierarchical model the preamble code in the generated function (where the observed are defined) gets dropped when I apply this transformation. Am I missing something? |
Describe the bug 🐞
Let us consider the following system:
We want to set the value of
k
tow
, changew
and then see thatk
got updated. We don't want changes to the original model, so any model changes have to be out of place.We can try this in 2 separate ways:
extend
ing the model with an additional one that adds the new defaults and transformsk
in a solvable parameter.defaults
andguesses
in-place.The first option would be:
Note that we have a couple of strange things happening:
k
inside the system, we see that we have the old valueThis results in the parameter update being ignored.
Now, for the second, we do it via via
deepcopy
:While this works as expected, the issue is that
deepcopy
sometimes triggers initialization issues for some reason, leading to #3307Expected behavior
A clear and concise description of what you expected to happen.
Minimal Reproducible Example 👇
Error & Stacktrace⚠️
There is no error, but the results are wrong since the parameter
k
is not solved for during initialization.Environment (please complete the following information):
using Pkg; Pkg.status()
using Pkg; Pkg.status(; mode = PKGMODE_MANIFEST)
versioninfo()
Additional context
A workaround to this issue is to use
deepcopy
, but that hits #3307.The text was updated successfully, but these errors were encountered: