Set u_modified in DAE initialization if the u was changed #1969
+9
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some integrators (such as Sundials) keep a shadow copy of `u` and need to reflect this copy back into the solver internals to use them. This happens e.g. during callbacks, but the same situation applies to initialization, so use the same mechanism (setting u_modified to `true`) to let the solver know that it needs to update its internal datastructures.
| @@ -183,6 +183,7 @@ function _initialize_dae!(integrator, prob::ODEProblem, alg::ShampineCollocation | |||
| integrator.u .= nlsol.u | |||
| failed = nlsol.retcode != ReturnCode.Success | |||
| end | |||
| u_modified!(integrator, true) | |||
There was a problem hiding this comment.
This is the default assumption. What's a case where this makes a difference?
|
Needs a test case |
Keno
added a commit
to Keno/Sundials.jl
that referenced
this pull request
Jun 17, 2023
Sundials.jl got an implementation of the OrdinaryDiffEq DAE initialization interface in SciML#401. Unfortunately, it turns out that this doesn't quite work, because the u/du in Sundials' integrator struct are not the actual memory that IDA uses (it has an internal copy). In [1], I propose re-using `u_modified` to inform us that the shadow copy was modified during initialization. This is the Sundials side of that interface. [1] SciML/OrdinaryDiffEq.jl#1969
Keno
added a commit
to Keno/Sundials.jl
that referenced
this pull request
Jun 17, 2023
Sundials.jl got an implementation of the OrdinaryDiffEq DAE initialization interface in SciML#401. Unfortunately, it turns out that this doesn't quite work, because the u/du in Sundials' integrator struct are not the actual memory that IDA uses (it has an internal copy). In [1], I propose re-using `u_modified` to inform us that the shadow copy was modified during initialization. This is the Sundials side of that interface. [1] SciML/OrdinaryDiffEq.jl#1969
ChrisRackauckas
pushed a commit
to Keno/Sundials.jl
that referenced
this pull request
Jul 1, 2023
Sundials.jl got an implementation of the OrdinaryDiffEq DAE initialization interface in SciML#401. Unfortunately, it turns out that this doesn't quite work, because the u/du in Sundials' integrator struct are not the actual memory that IDA uses (it has an internal copy). In [1], I propose re-using `u_modified` to inform us that the shadow copy was modified during initialization. This is the Sundials side of that interface. [1] SciML/OrdinaryDiffEq.jl#1969
ChrisRackauckas
pushed a commit
to SciML/Sundials.jl
that referenced
this pull request
Jul 1, 2023
Sundials.jl got an implementation of the OrdinaryDiffEq DAE initialization interface in #401. Unfortunately, it turns out that this doesn't quite work, because the u/du in Sundials' integrator struct are not the actual memory that IDA uses (it has an internal copy). In [1], I propose re-using `u_modified` to inform us that the shadow copy was modified during initialization. This is the Sundials side of that interface. [1] SciML/OrdinaryDiffEq.jl#1969
|
I don't believe this is necessary. Specifically, setting u_modified in inits is only needed for Sundials solvers, and sundials solvers don't use ODE/DAEIntegrator so these initialization methods will never be used with them, |
That's true, but only by default, there's nothing preventing people from setting these manually or chaining them, but even if nobody were to ever do that, that's not really the point. The question is whether the interface requires these to be set and as written, the answer is yes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some integrators (such as Sundials) keep a shadow copy of
uand need to reflect this copy back into the solver internals to use them. This happens e.g. during callbacks, but the same situation applies to initialization, so use the same mechanism (setting u_modified totrue) to let the solver know that it needs to update its internal datastructures.