-
-
Notifications
You must be signed in to change notification settings - Fork 217
fix: copy initials to u0
if u0
not provided to remake
#3572
New issue
Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “No 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? No Sign in to your account
fix: copy initials to u0
if u0
not provided to remake
#3572
Conversation
7aa8c0c
to
8a19a49
Compare
@@ -658,6 +659,18 @@ function SciMLBase.late_binding_update_u0_p( | |||
prob, sys::AbstractSystem, u0, p, t0, newu0, newp) | |||
supports_initialization(sys) || return newu0, newp | |||
u0 === missing && return newu0, (p === missing ? copy(newp) : newp) | |||
# If the user passes `p` to `remake` but not `u0` and `u0` isn't empty, | |||
# and if the system supports initialization (so it has initial parameters), | |||
# and if the initialization solves for `u0`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't know this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do. Initialization solves for unknowns if initializeprobmap !== nothing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't know what the initialization will be until solve
is called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have all the information about the initialization in the ODEProblem
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't know which initialization method will be used until later. CheckInit and NoInit don't solve for u0.
ddc22ca
to
c385c11
Compare
Waiting for SciML/DiffEqBase.jl#1143 |
f788eeb
to
0aa0d8b
Compare
getter = if meta isa InitializationMetadata | ||
meta.get_initial_unknowns | ||
else | ||
getu(sys, Initial.(unknowns(sys))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this will get the guesses too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would the guesses come into the picture here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same reason as https://github.com/SciML/SciMLSensitivity.jl/pull/1168/files#diff-76127b1004a68143584d305e1c9ee36126238fd5a05ddeff5985f5b5450ed2c2R92, the u0
vector on DAEs is canonically interpreted as either the initial value or the guess for u0
, depending on the choice of initialization and which one exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. That's a little more annoying, but not difficult to do. I'll fix it so this propagates guesses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed now
0aa0d8b
to
1450a5f
Compare
Cancelled CI because I forgot to add a test |
a611042
to
997da8d
Compare
SciMLBase/Downstream failure is SciML/SciMLBase.jl#1006. |
Close #3570