-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Different solutions for in-place and not in-place functions with unconstrained algorithm #15
Comments
I think that shoiuld work? Of course, if the tests fail after doing that then the answer is no. Likely when first implementing it I added
It needs to update the current interpolation to use the new values of
Looks like it can be removed and was part of a bad attempt at getting this to work.
That looks like a mistake and could be the root of the issue. |
For ODEs with BS3: does inplace and out of place give the exact same answer on a size-1 array? I believe the answer should be yes but that's the first thing to check. |
So for the interpolation to work correctly, it is not sufficient to update
In the example above the functions of problems |
This probably needs an extra comment. It's more like this. When dt > smallest delay, the history function is implicit and requires values in
So it's moreso that the domain of interpolation needs to change after the first iteration. That is the part that is tricky and is why all of those
I would still check the ODE solver works out here like you think it does. I am 90% certain it does, but I don't think I CI test every solver to make sure it does. |
Thanks for the explanation! It makes sense, that's the way I understood it mathematically, as well. However, I'm not sure I got it how exactly an
Sorry, I misread it in the first place, I didn't get that you were talking about ODE solvers. Yes, this was one of the first things I checked and the ODE solver seems to work in the same way for both in-place and not in-place functions: julia> using DiffEqProblemLibrary, OrdinaryDiffEq
julia> alg = BS3()
julia> prob_notinplace = ODEProblem(DiffEqProblemLibrary.linear, 1.0, (0.0, 5.0))
julia> prob_inplace = ODEProblem(DiffEqProblemLibrary.f_2dlinear, [1.0], (0.0, 5.0))
julia> sol_notinplace = solve(prob_notinplace, alg)
julia> sol_inplace = solve(prob_inplace, alg)
julia> sol_notinplace.t == sol_inplace.t
true
julia> sol_notinplace.u == sol_inplace[1, :]
true
julia> sol_notinplace.errors
Dict{Symbol,Float64} with 3 entries:
:l∞ => 0.311186
:final => 0.311186
:l2 => 0.0920546
julia> sol_inplace.errors
Dict{Symbol,Float64} with 3 entries:
:l∞ => 0.311186
:final => 0.311186
:l2 => 0.0920546 |
Coming back to the question and remarks from above:
All tests still pass after applying those changes, and also my example still fails (with the same errors).
Same here, changed code passes tests, but my example still fails.
Tried to update those values in So I think the root of the problem actually might be
I see (at least) the following problems:
|
Yes, now that I think about it, that's it. That's actually really bad. A good example which would break this is the SSPRK methods. In those methods, that updates At that function call, it will use the updated history function because we have set the pointers and not the values the same. That for sure will cause a difference. Here that's slightly beneficial (updated And now I see you noticed the same thing 👍 . |
I already tried to separate and hence it's not possible to use since both integrators point to the same https://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L133. The second option seems easier, but I'm not sure whether a separate |
No, a separate
A separate |
I'm not sure whether I understand you correctly, but I guess the problem I'm dealing with can't be fixed by an initialization of
I tried to implement it similarly in the beginning, but somehow I still ended up with different time points and a small difference in the error estimates (same example as above): julia> sol.errors
Dict(:l∞=>3.6147e-5,:final=>1.91515e-5,:l2=>1.49162e-5)
julia> sol2.errors
Dict(:l∞=>3.61461e-5,:final=>1.91514e-5,:l2=>1.49159e-5) Hence I was worried there might be different variables pointing to the same array that somehow lead to a different result for in-place functions compared to not in-place functions. The only other thing I could think of was that the call to I tried also an implementation with two separate caches, but (as expected with the reasoning above) the example fails with the same error estimates. So the still existent discrepancy might be due to a completely different problem, but I'm out of ideas at the moment. Nevertheless, I'll have a look at my code again, and I can also upload a simple version which just copies |
Integrators with lazy evaluation of extra steps just won't work right now: #16. So that can be dealt with separately in the near future.
PR what you have there since it's a good improvement already. As for the last little bit, the best way is to print everything out in the first step and find out what the first term that's calculated differently is. |
Hi!
As already mentioned in #14, the unconstrained algorithm computes different solutions for problems with in-place and not in-place functions. A small example:
I assume the problem originates from the DDE
integrator
and the ODEintegrator.integrator
pointing, among others, both to the same arraysu
,uprev
, andk
in the in-place case. So for some assignments such as https://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L96, which is executed in the first iteration, the value is only updated once for not in-place integrators but immediately in every step for in-place integrators. Moreover, even during calculations in https://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L69 any change ofu
ork
is immediately propagated tointegrator.integrator
, and thus might also change the interpolation which is not desired, I guess. I was able to reduce the difference of the errors in the example above by completely separatingintegrator.u
andintegrator.k
also for in-place functions, i.e. by creating a copy of both arrays during initialization and always usingrecursivecopy!
instead of assignments such asintegrator.integrator.u = integrator.u
, but I got still a tiny difference.However, I'm still confused by the current implementation of
perform_step!
. For step sizes below the minimal delay the next step can be directly calculated by the correspond method inOrdinaryDiffEq.jl
(https://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L114), whereas for larger step sizes one calculates a first approximation of the next step (also viaOrdinaryDiffEq.jl
, https://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L69), which is then used for interpolation during the next iteration sinceintegrator.f
depends on aHistoryFunction
that contains the initial history function, the current solution ofintegrator.integrator
, and the current state ofintegrator.integrator
(which is relevant for interpolation). If this is correct, I have some questions/remarks:integrator.integrator.tprev = integrator.t
? To enable extrapolation in the first iteration onlyintegrator.integrator
is relevant, so this should be sufficient? And we can remove the redundant line https://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L94, can't we?integrator.uprev
andintegrator.integrator.uprev
not always equal the same value since also in subsequent iterations the value at timet
of the integrator, that was reached in the last step, stays the same? So why do we needhttps://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L96
?integrator.t
,integrator.tprev
, andintegrator.uprev
, even though they are not changed byperform_step!
inOrdinaryDiffEq.jl
? Can't we remove lines https://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L54-L56 and https://github.com/JuliaDiffEq/DelayDiffEq.jl/blob/master/src/integrator_interface.jl#L107-L109?u
,k
,fsallast
, andEEst
that are changed byperform_step!
inintegrator
not copied tointegrator.integrator
at the end of the method?The text was updated successfully, but these errors were encountered: