Skip to content
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.

Sign up for GitHub

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

Compute third stage time-step for RK3 in a way that reduces the accumulation of error #3617

Merged
merged 14 commits into from
Jun 13, 2024
Merged

Compute third stage time-step for RK3 in a way that reduces the accumulation of error #3617

Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5077631
Change TimeInterval to avoid roundoff error issue plus some cleanup
glwagner Jun 6, 2024
2a28af3
fix typo
glwagner Jun 6, 2024
4642bfa
Import fix
glwagner Jun 6, 2024
7e099ee
Potentially address overflow problem
glwagner Jun 6, 2024
8b8a426
Recompute new third stage time-step to reduce error accumulation
glwagner Jun 6, 2024
e76934d
Make schedule work even with fake models
glwagner Jun 6, 2024
b0b35d3
Merge branch 'glw/time-interval-fix' into glw/reduce-time-error-accum…
glwagner Jun 6, 2024
e5dbae3
Fix tests for TimeInterval
glwagner Jun 6, 2024
bf2298d
Fix JLD2 output writer tests
glwagner Jun 6, 2024
a7628a4
Fix NetCDF writer test?
glwagner Jun 6, 2024
64bc964
Merge branch 'glw/time-interval-fix' into glw/reduce-time-error-accum…
glwagner Jun 6, 2024
05e41e8
Merge branch 'main' into glw/reduce-time-error-accumulation
glwagner Jun 7, 2024
4ca30ab
Merge branch 'main' into glw/reduce-time-error-accumulation
glwagner Jun 8, 2024
2e0811a
Merge branch 'main' into glw/reduce-time-error-accumulation
glwagner Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/TimeSteppers/runge_kutta_3.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ function time_step!(model::AbstractModel{<:RungeKutta3TimeStepper}, Δt; callbac
second_stage_Δt = (γ² + ζ²) * Δt
third_stage_Δt = (γ³ + ζ³) * Δt

# Compute the next time step a priori to reduce floating point error accumulation
tⁿ⁺¹ = next_time(model.clock, Δt)

#
# First stage
#
Expand Down Expand Up @@ -132,7 +135,10 @@ function time_step!(model::AbstractModel{<:RungeKutta3TimeStepper}, Δt; callbac
calculate_pressure_correction!(model, third_stage_Δt)
pressure_correct_velocities!(model, third_stage_Δt)

tick!(model.clock, third_stage_Δt)
# This formulation of the final time-step reduces the accumulation of round-off error when Δt
# is added to model.clock.time.
corrected_third_stage_Δt = tⁿ⁺¹ - model.clock.time
tick!(model.clock, corrected_third_stage_Δt)
update_state!(model, callbacks; compute_tendencies)
step_lagrangian_particles!(model, third_stage_Δt)
Comment on lines +140 to 143
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we tick! with corrected_third_stage_Δt, but other third stage steps (namely we calculate_pressure_correction!, pressure_correct_velocities! and step_langrangian_particles!) use the original third_stage_Δt. I would assume all of them need to use the corrected version, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm we can change that though, though it shouldn't matter because they are indistinguishable except by machine epsilon?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right I did consider this. We can change the pressure correction and lagrangian particles. However, other tracers effectively use the original third stage dt via rk3 substep. So I think it's actually simpler to use the same third_state_dt for everything (tracers, predictor velocity, pressure correction, lagrangian particles), but to simply shave off the error accumulated in the model clock during the substeps

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm we can change that though, though it shouldn't matter because they are indistinguishable except by machine epsilon?

Yeah, it's more a matter of consistency than anything else imo

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, so my point is that it's more consistent to use the non-corrected time-step everywhere, including rk3substep! (where it is used implicitly), as well as in the pressure correction and Lagrangian particles step. Do you agree?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see what you're saying. Yes I agree.


Expand Down