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
WIP: implement fifth order Radau IIA #612
Conversation
YingboMa
commented
Jan 20, 2019
Codecov Report
@@ Coverage Diff @@
## master #612 +/- ##
==========================================
+ Coverage 68.45% 68.62% +0.17%
==========================================
Files 83 86 +3
Lines 24721 24883 +162
==========================================
+ Hits 16923 17077 +154
- Misses 7798 7806 +8
Continue to review full report at Codecov.
|
The adaptivity of the current implementation is very weird. julia> using OrdinaryDiffEq, DiffEqDevTools, Test, LinearAlgebra
julia> using DiffEqProblemLibrary.ODEProblemLibrary: importodeproblems; importodeproblems()
julia> import DiffEqProblemLibrary.ODEProblemLibrary: van
julia> solve(remake(vanstiff, p=1e7), RadauIIA5(), abstol=1e-5, reltol=1e-7) |> length
668
julia> solve(remake(vanstiff, p=1e7), RadauIIA5()) |> length
┌ Warning: Interrupted. Larger maxiters is needed.
└ @ DiffEqBase ~/.julia/packages/DiffEqBase/8usQ9/src/integrator_interface.jl:150
95
julia> solve(remake(vanstiff, p=1e7), Kvaerno5()) |> length
110
julia> solve(remake(vanstiff, p=1e7), Kvaerno5(), abstol=1e-5, reltol=1e-7) |> length
401 |
|
||
if adaptive | ||
e1dt, e2dt, e3dt = e1/dt, e2/dt, e3/dt | ||
tmp = @. e1dt*z1 + e2dt*z2 + e3dt*z3 |
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 this not the err tilde term?
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.
The err tilde term is integrator.fsalfirst + tmp
.
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.
e1dt, e2dt, e3dt = e1/dt, e2/dt, e3/dt | ||
tmp = @. e1dt*z1 + e2dt*z2 + e3dt*z3 | ||
mass_matrix != I && (tmp = mass_matrix*tmp) | ||
err = @. integrator.fsalfirst + tmp |
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.
shouldn't this not be err tilde?
γdt, αdt, βdt = γ/dt, α/dt, β/dt | ||
J = calc_J(integrator, cache, is_compos) | ||
if u isa Number | ||
LU1 = γdt*mass_matrix - J |
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 term should be mass_matrix/(gamma*dt), but it's not
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.
nevermind
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.
wait, yes it is.
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.
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.
In all cases you should have gamma*dt
. The transformation is to divide both sides by gdt = gamma*dt
. The reason is because J
is O(n^2) but I
is O(n)
(without mass matrices of course), so this decreases the overall calculations. This is exactly the same as the invW_t
transformation on the Rosenbrock and SDIRK methods.
src/integrators/integrator_utils.jl
Outdated
qtmp = (integrator.EEst^expo)/fac | ||
@fastmath q = max(inv(integrator.opts.qmax),min(inv(integrator.opts.qmin),qtmp)) | ||
integrator.qold = q | ||
if q <= integrator.opts.qsteady_max && q >= integrator.opts.qsteady_min |
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 is supposed to be handling
But it looks slightly incorrect. It's really only used with a few recent algorithms:
https://github.com/JuliaDiffEq/OrdinaryDiffEq.jl/blob/master/src/alg_utils.jl#L312-L321
This might be the main difference between us and CVODE. Check with LSODE stuff and see if they are using qold
here like Hairer.
u = @. uprev + z3 | ||
|
||
if adaptive | ||
e1dt, e2dt, e3dt = e1/dt, e2/dt, e3/dt |
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.
These should all be divided by γdt
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.