-
-
Notifications
You must be signed in to change notification settings - Fork 214
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
Adaptive explicit extrapolation #714
Adaptive explicit extrapolation #714
Conversation
…thm ExtrapolationMidpointDeuflhard
…xtrapolation_caches.jl. Small adjustments in algorithms.jl and alg_utils.jl
Integration of `ExtrapolationMidpointDeuflhard` algorithm using a constant cache. Stepsize control does not work properly.
ExtrapolationMidpointDeuflhard() with a constant cache.
…and ExtrapolationMidpointHairerWanner
@@ -339,16 +346,22 @@ beta2_default(alg::FunctionMap) = 0 | |||
beta2_default(alg::DP8) = 0//1 | |||
beta2_default(alg::DP5) = 4//100 | |||
beta2_default(alg::DP5Threaded) = 4//100 | |||
beta2_default(alg::ExtrapolationMidpointDeuflhard) = 0//1 |
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.
Is there an issue with PI adaptivity here?
subdividing_sequence::Array{BigInt,1} # subdividing_sequence[n] is used for the (n -1)th internal discretisation | ||
stage_number::Array{Int,1} # Stage_number[n] contains information for extrapolation order (n + alg.n_min - 1) | ||
# Weights and Scaling factors for extrapolation operators | ||
extrapolation_weights::Array{Rational{BigInt},2} |
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.
Do these need to be Rational? Maybe it doesn't matter all that much, but Bigs are allocating, so it would be worthwhile to profile if the calculations on these actually end up being costly, and whether just having them as Float64 may be numerically stable enough to get the job done. If Float64 is enough, then typing these to make t
or u
would make sense.
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 haven't played with Floats here so far.
The good thing about theBigInt
s is that the weights are converted in perform_step
to the type of u
. That allows to adapt the precision of the problem.
I'd suspect that if one uses Float64
weights and say aBigFloat
representation for the problem, the approximation error will not get any better than eps(Float64)
.
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.
What about just converting to u
's type 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.
Ah I see - the caches are recreated for every call of solve
, right? Yes, then the type of u
should be an option.
I guess I will set up an experiment to check the stability of the weights!
BTW: Would it be possible to move the weights into the algorithm's struct
? That way the computation of the tables for every solve
call would be bypassed..
src/caches/extrapolation_caches.jl
Outdated
function alg_cache(alg::ExtrapolationMidpointDeuflhard,u,rate_prototype,uEltypeNoUnits,uBottomEltypeNoUnits,tTypeNoUnits,uprev,uprev2,f,t,dt,reltol,p,calck,::Type{Val{false}}) | ||
@unpack n_min, n_init, n_max = alg | ||
|
||
QType = tTypeNoUnits <: Integer ? typeof(qmin_default(alg)) : tTypeNoUnits # Cf. DiffEqBase.__init in solve.jl |
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.
peculiar that the type of this is needed. I'll see where this goes...
|
||
# Compute scaling factors for extrapolation operators | ||
extrapolation_scalars = -nodes[1] * [BigInt(1); extrapolation_scalars_2] | ||
|
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.
Because the amount of pre-computations is based on n_max
, it actually matters in small enough settings. That is something that may be worth remembering to document, or at least test a little bit. BTW, at when n_max
does overflow start to occur in the sequences?
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.
Ok, I will mention this in the documentation.
As far as I've tested the constructor, I have not been able to produce an overflow (for any sequence) if using BigInt
. As I replaced BigInt
by Int
, I got an overflow for the Romberg sequence and n_max = 6
extrapolation_scalars_2::Array{Rational{BigInt},1} | ||
end | ||
|
||
function alg_cache(alg::ExtrapolationMidpointHairerWanner,u,rate_prototype,uEltypeNoUnits,uBottomEltypeNoUnits,tTypeNoUnits,uprev,uprev2,f,t,dt,reltol,p,calck,::Type{Val{false}}) |
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 feel like some refactoring can make this re-use a lot of code from the previous cache function. That's a second order issue that we can handle later though.
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.
That's true. Am I "allowed" to define a function producing the weights etc. somewhere? If so where?
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.
just do it in another part of this file, and then call it from in 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.
Ok I will change that!
src/caches/extrapolation_caches.jl
Outdated
cc = alg_cache(alg,u,rate_prototype,uEltypeNoUnits,uBottomEltypeNoUnits,tTypeNoUnits,uprev,uprev2,f,t,dt,reltol,p,calck,Val{false}) | ||
|
||
ExtrapolationMidpointDeuflhardCache(u_tilde, u_temp1, u_temp2, tmp, T, res, fsalfirst, k, | ||
cc.Q, cc.n_curr, cc.n_old, cc.subdividing_sequence, cc.stage_number, cc.extrapolation_weights, cc.extrapolation_scalars, cc.extrapolation_weights_2, cc.extrapolation_scalars_2) |
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 might be easier to just stick the cc
in here, using composition to improve code re-use. But again with the other comment, optimizing code simplicity and re-use can come in a later PR.
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.
Here I decided against that to make the call @unpack subdividing_sequence = cache
and such possible for both caches.
Would you prefer an if - else
statement before calling @unpack
in order to check if a cache is mutable or not?
src/integrators/integrator_utils.jl
Outdated
integrator.opts.beta1 = typeof(integrator.opts.beta1)(1 // (2integrator.cache.n_curr + 1)) | ||
integrator.opts.gamma = typeof(integrator.opts.gamma)(1 // 4)^integrator.opts.beta1 | ||
# Compute new stepsize scaling | ||
qtmp = integrator.EEst^integrator.opts.beta1 / integrator.opts.gamma |
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 why you got rid of beta2. Something that would be worthwhile later would be to try and make this use PI instead of P, since step stabilization will help a method based on explicit RKs take larger steps.
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.
But hmm, yes anything based on a work estimate is hard to stabilize.
src/integrators/integrator_utils.jl
Outdated
else | ||
# Initialize | ||
@unpack t,EEst = integrator | ||
tol = integrator.opts.internalnorm(integrator.opts.reltol,t) # Deuflhard's approach relies on EEstD ≈ ||relTol|| |
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.
This might have weird interactions with element-wise tolerances, but does work.
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.
That is a problem I have not figured out yet. The thing is that in Deuflhard's approach some sort of notion of a relative tolerance (< 1 !) is needed for the convergence monitor and stepsize prediction.
I guess this forces the user in the case of vector valued rel. and abs. tolerance to think of these vectors as scaling vectors multiplied by tol
.
So far I haven't played with element-wise tolerances and don't know what the effects are...
What's the reference you used for the stepping behaviors? |
What do you mean exactly - the literature for the adaptivity? |
What's the literature you used for the implementation? That's what I meant. |
My two main sources have been the book of Hairer, Wanner and Nørsett (Solving Ordinary Differential Equations I) |
Does this also apply to expressions like |
Thank you! Why this behavior depends on the chosen subdividing sequence is not really clear to me... |
All if possible. |
I handled the merge conflicts. We may want to just merge the PR once I check out the convergence tests are all good, and then future updates can take place in follow up PRs. It's much easier to keep doing small PRs than it is to keep a branch from going stale. |
Codecov Report
@@ Coverage Diff @@
## master #714 +/- ##
==========================================
- Coverage 72.01% 71.63% -0.39%
==========================================
Files 93 93
Lines 28806 29247 +441
==========================================
+ Hits 20746 20952 +206
- Misses 8060 8295 +235
Continue to review full report at Codecov.
|
My guess is that something odd happens in a spot like
I'll merge if tests pass and we can continue from there. Thanks! This is quite a nice PR, and we will want to continue down the path of https://arxiv.org/abs/1305.6165 . With Julia's new multithreading system this will be pretty exciting. One contribution you may want to think about is the implicit methods. Since you have the sequences now, swapping out for the implicit parts shouldn't be too bad: just use the nonlinear solvers already in OrdinaryDiffEq. Doing this with the parallelism was attempted in Python, but of course Python got in the way: numerical-mathematics/extrapolation#27 . @YingboMa and I have interesting test cases where BLAS multithreading doesn't speed up the calculation (and in fact slows it down), so taking better control of the parallelism can help on some 50ish ODE very implicit systems. |
This commit introduced a binary and needs to be deleted somehow: 6ddcfcb , otherwise it will sit and clog and repo history. |
f0a3cbf
to
e080e64
Compare
I cleaned it up and removed the binary from the history. If all went well, then tests will pass again and I'll merge. |
Okay, thank you so much for all the help! |
Yes
Everything is moving away from notebooks. We're in the middle of a benchmark overhaul. See SciML/SciMLBenchmarks.jl#32 |
|
This are two implementations of the adaptive extrapolation of the explicit midpoint rule (Similar to
ODEX
andDIFEX
).I developed the original code for my bachelor's thesis and would like to add it now to the JuliaDiffEq ecosystem.
This is the first working version of the algorithms and I would love to get some feedback:
Are my additions ok? Is there something I have to change? Are there some essential options from the common interface I should implement?
Since it is the first time that I contribute to an open source project, I would also be grateful to get some tips, which tests I should implement.
Thanks!