Skip to content
46 changes: 46 additions & 0 deletions examples/kdv_1d/kdv_1d_IMEX.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
using OrdinaryDiffEqTsit5
using DispersiveShallowWater
using SummationByPartsOperators: upwind_operators, periodic_derivative_operator

###############################################################################
# Semidiscretization of the KdV equation

equations = KdVEquation1D(gravity = 9.81, D = 1.0)
initial_condition = initial_condition_convergence_test
boundary_conditions = boundary_condition_periodic

# create homogeneous mesh
coordinates_min = -50.0
coordinates_max = 50.0
N = 512
mesh = Mesh1D(coordinates_min, coordinates_max, N)

# Create solver with periodic SBP operators of accuracy order 3,
# which results in a 4th order accurate semi discretizations.
# We can set the accuracy order of the upwind operators to 3 since
# we only use central versions/combinations of the upwind operators.
D1_upwind = upwind_operators(periodic_derivative_operator;
derivative_order = 1, accuracy_order = 3,
xmin = xmin(mesh), xmax = xmax(mesh),
N = nnodes(mesh))
solver = Solver(D1_upwind)

semi = Semidiscretization(mesh, equations, initial_condition, solver,
boundary_conditions = boundary_conditions)

tspan = (0.0, 5.0)
ode = semidiscretize(semi, tspan, no_splitform = false)

summary_callback = SummaryCallback()
analysis_callback = AnalysisCallback(semi; interval = 100,
extra_analysis_errors = (:conservation_error,),
extra_analysis_integrals = (waterheight_total,
waterheight))
callbacks = CallbackSet(analysis_callback, summary_callback)
saveat = range(tspan..., length = 100)

# alg = Rodas5() # not working because of https://github.com/SciML/OrdinaryDiffEq.jl/issues/2719
# alg = KenCarp4() # would need to add OrdinaryDiffEqSDIRK - can to that, but I dont want to bloat DSW.jl with package s
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to use a true IMEX scheme like KenCarp4 here. Something like Tsit5 will use explicit time stepping also for the stiff part, right? I think it's fine to just add OrdinaryDiffEqSDIRK.jl to the test suite. It doesn't need to be a dependency of DSW.jl.

alg = Tsit5()
sol = solve(ode, alg, abstol = 1e-7, reltol = 1e-7,
save_everystep = false, callback = callbacks, saveat = saveat)
2 changes: 1 addition & 1 deletion src/DispersiveShallowWater.jl
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ using RecursiveArrayTools: ArrayPartition
using Reexport: @reexport
using Roots: AlefeldPotraShi, find_zero

using SciMLBase: SciMLBase, DiscreteCallback, ODEProblem, ODESolution
using SciMLBase: SciMLBase, DiscreteCallback, ODEProblem, ODESolution, SplitFunction
import SciMLBase: u_modified!

@reexport using StaticArrays: SVector
Expand Down
69 changes: 69 additions & 0 deletions src/equations/kdv_1d.jl
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,72 @@ function rhs!(dq, q, t, mesh, equations::KdVEquation1D, initial_condition,

return nothing
end

function rhs_split_1!(dq, q, t, mesh, equations::KdVEquation1D, initial_condition,
::BoundaryConditionPeriodic, source_terms, solver, cache)
eta, = q.x
deta, = dq.x

(; c_0, DD) = cache
# In order to use automatic differentiation, we need to extract
# the storage vectors using `get_tmp` from PreallocationTools.jl
# so they can also hold dual numbers when needed.
tmp_1 = get_tmp(cache.tmp_1, eta)
tmp_2 = get_tmp(cache.tmp_2, eta)

@trixi_timeit timer() "third-order derivatives" begin
if solver.D1 isa PeriodicUpwindOperators && isnothing(solver.D3)
# eta_xxx = Dp * Dc * Dm * eta
mul!(tmp_1, solver.D1.minus, eta)
mul!(tmp_2, solver.D1.central, tmp_1)
mul!(tmp_1, solver.D1.plus, tmp_2)
else
# eta_xxx = D3 * eta
mul!(tmp_1, solver.D3, eta)
end

# deta = 1 / 6 sqrt(g * D) D^2 eta_xxx

@.. deta = -1 / 6 * c_0 * DD * tmp_1
end

return nothing
end

function rhs_split_2!(dq, q, t, mesh, equations::KdVEquation1D, initial_condition,
::BoundaryConditionPeriodic, source_terms, solver, cache)
eta, = q.x
deta, = dq.x

(; c_0, c_1) = cache
# In order to use automatic differentiation, we need to extract
# the storage vectors using `get_tmp` from PreallocationTools.jl
# so they can also hold dual numbers when needed.
tmp_1 = get_tmp(cache.tmp_1, eta)
tmp_2 = get_tmp(cache.tmp_2, eta)

if solver.D1 isa PeriodicUpwindOperators && isnothing(solver.D3)
D1 = solver.D1.central
else
D1 = solver.D1
end

@trixi_timeit timer() "hyperbolic" begin
# eta2 = eta^2
@.. tmp_1 = eta^2

# eta2_x = D1 * eta2
mul!(tmp_2, D1, tmp_1)

# eta_x = D1 * eta
mul!(tmp_1, D1, eta)

# deta -= sqrt(g * D) * eta_x + 1 / 2 * sqrt(g / D) * (eta * eta_x + eta2_x)
@.. deta = -(c_0 * tmp_1 + c_1 * (eta * tmp_1 + tmp_2))
end

@trixi_timeit timer() "source terms" calc_sources!(dq, q, t, source_terms, equations,
solver)

return nothing
end
36 changes: 32 additions & 4 deletions src/semidiscretization.jl
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,28 @@ function rhs!(dq, q, semi::Semidiscretization, t)
return nothing
end

function rhs_split_1!(dq, q, semi::Semidiscretization, t)
@unpack mesh, equations, initial_condition, boundary_conditions, solver, source_terms, cache = semi

@trixi_timeit timer() "rhs_split_1!" rhs_split_1!(dq, q, t, mesh, equations,
initial_condition,
boundary_conditions, source_terms,
solver,
cache)
return nothing
end

function rhs_split_2!(dq, q, semi::Semidiscretization, t)
@unpack mesh, equations, initial_condition, boundary_conditions, solver, source_terms, cache = semi

@trixi_timeit timer() "rhs_split_2!" rhs_split_2!(dq, q, t, mesh, equations,
initial_condition,
boundary_conditions, source_terms,
solver,
cache)
return nothing
end

function compute_coefficients(func, t, semi::Semidiscretization)
@unpack mesh, equations, solver = semi
q = allocate_coefficients(mesh_equations_solver(semi)...)
Expand Down Expand Up @@ -214,16 +236,21 @@ function check_bathymetry(equations::AbstractShallowWaterEquations, q0)
end

"""
semidiscretize(semi::Semidiscretization, tspan)
semidiscretize(semi::Semidiscretization, tspan; no_splitform = true)

Wrap the semidiscretization `semi` as an ODE problem in the time interval `tspan`
that can be passed to `solve` from the [SciML ecosystem](https://diffeq.sciml.ai/latest/).
"""
function semidiscretize(semi::Semidiscretization, tspan)
function semidiscretize(semi::Semidiscretization, tspan; no_splitform = true)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering what's the best interface for this. I initially thought it would be good to add a trait has_stiff_terms(equations), which can be used to determine whether we create a usual ODEFunction or a SplitFunction. This way, we wouldn't need a kwarg no_splitform or similar, but whether or not using the SplitFunction formulation will be determined from the equations. However, this would mean that we also use a split function in the case of a non-IMEX scheme, which could have some overhead (I think that's a similar approach as Trixi.jl does with periodic equations, right @ranocha?). Maybe you could first add one or two KdV examples to the benchmark suite as in the list of #202, such that we can try how much overhead we would have from the SplitFunction formulation for the explicit solvers?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The advantage of the has_stiff_terms (or maybe better have_stiff_terms because equations are plural) would be:

  • the user would not need to set no_splitform = false in semidiscretize in order to use an IMEX scheme
  • we wouldn't need the rhs! definition anymore because we always define a SplitFunction for equations, which have_stiff_terms reducing the maintenance overhead and avoiding duplication.

The disadvantage I see is

  • maybe a small overhead.

Any more aspects to consider?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hyperbolic approximations would be tricky since they can have quite stiff terms depending on the hyperbolic approximation parameter (lambda for the hyperbolic SGN equations). Still, we may also want to use them with explicit time integration methods as efficiently as possible.
Currently, "as efficiently as possible" is significantly restricted by SciML/RecursiveArrayTools.jl#400, so I am unsure of its importance. We should benchmark it.
To sum up, we could add a trait have_stiff_terms to the equations and introduce a keyword argument like split_ode = have_stiff_terms(equations). We should ensure that this results in a type-stable semidiscretize, i.e., the keyword argument can take True() and False() values (with different types).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. So if I understand correctly, we want to support both the classical ODEFunction version and the SplitODEFunction version, i.e. we also need to keep both rhs! and rhs_split_stiff! and rhs_split_nonstiff!. Is there an elegant way to reduce code duplication and make maintenance easier because as Collin wrote in the initial comment of the PR we cannot simply call both versions after each other in rhs!.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could also pass an argument to rhs_split_nonstiff! which then decides of it should be deta = ... or deta += ... which if known at compile time would mean no overhead (and otherwise just an if statement) and it would make it possible to have

function rhs!(...)
   rhs_split_stiff!(...)
   rhs_split_nonstiff!(... :add)
end

afterall.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do it like this: since we only support the KdV equation with implicit methods so far, we can just hard-code that. For the hyperbolic approximation of SGN, we keep the current structure and create an issue to let us reconsider this in the future.

q0 = compute_coefficients(semi.initial_condition, first(tspan), semi)
check_bathymetry(semi.equations, q0)
iip = true # is-inplace, i.e., we modify a vector when calling rhs!
return ODEProblem{iip}(rhs!, q0, tspan, semi)
if no_splitform
ode = ODEProblem{iip}(rhs!, q0, tspan, semi)
else
ode = ODEProblem{iip}(SplitFunction(rhs_split_1!, rhs_split_2!), q0, tspan, semi)
end
return ode
end

"""
Expand All @@ -241,7 +268,8 @@ of the semidiscretization `semi` at the state `q0`.
function jacobian(semi::Semidiscretization;
t = 0.0,
q0 = compute_coefficients(semi.initial_condition, t, semi))
J = ForwardDiff.jacobian(similar(q0), q0) do dq, q
@unpack tmp_partitioned = semi.cache
J = ForwardDiff.jacobian(tmp_partitioned, q0) do dq, q
DispersiveShallowWater.rhs!(dq, q, semi, t)
end
return J
Expand Down
11 changes: 11 additions & 0 deletions test/test_kdv_1d.jl
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,14 @@ end

@test_allocations(semi, sol, allocs=5_000)
end

@testitem "kdv_1d_IMEX" setup=[Setup, KdVEquation1D] begin
@test_trixi_include(joinpath(EXAMPLES_DIR, "kdv_1d_IMEX.jl"),
tspan=(0.0, 5.0),
l2=[0.0007835879713461127],
linf=[0.0005961613764722262],
cons_error=[4.440892098500626e-16],
change_waterheight=-4.440892098500626e-16)

@test_allocations_splitform(semi, sol, allocs=5_000)
end
18 changes: 18 additions & 0 deletions test/test_util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,24 @@ macro test_allocations(semi, sol, allocs)
end
end

"""
@test_allocations_splitform(semi, sol, allocs)

Test that the memory allocations of `DispersiveShallowWater.rhs_split_1!`
and `DispersiveShallowWater.rhs_split_2!` are below `allocs`
(e.g., from type instabilities).
"""
macro test_allocations_splitform(semi, sol, allocs)
quote
t = $sol.t[end]
q = $sol.u[end]
dq = similar(q)
a1 = @allocated DispersiveShallowWater.rhs_split_1!(dq, q, $semi, t)
a2 = @allocated DispersiveShallowWater.rhs_split_2!(dq, q, $semi, t)
@test (a1 + a2) < $allocs
end
end

macro test_nowarn_mod(expr, additional_ignore_content = [])
quote
add_to_additional_ignore_content = [
Expand Down
Loading