From 5077631b1d77cc35f10727ce3e8afbb8e9dfdc4e Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 14:00:13 +0200 Subject: [PATCH 1/9] Change TimeInterval to avoid roundoff error issue plus some cleanup --- src/Simulations/callback.jl | 3 ++- src/Simulations/run.jl | 6 ++--- src/Utils/schedules.jl | 48 +++++++++++++++++++++++-------------- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/Simulations/callback.jl b/src/Simulations/callback.jl index dfc4dd905f..4956ca3c0e 100644 --- a/src/Simulations/callback.jl +++ b/src/Simulations/callback.jl @@ -14,7 +14,8 @@ end @inline (callback::Callback)(sim) = callback.func(sim, callback.parameters) @inline (callback::Callback{<:Nothing})(sim) = callback.func(sim) -# Fallback initialization: call the schedule, then the callback +# Fallback initialization: initialize the schedule. +# Then, if the schedule calls for it, execute the callback. function initialize!(callback::Callback, sim) initialize!(callback.schedule, sim.model) && callback(sim) return nothing diff --git a/src/Simulations/run.jl b/src/Simulations/run.jl index 56c46de757..591d249f59 100644 --- a/src/Simulations/run.jl +++ b/src/Simulations/run.jl @@ -21,12 +21,12 @@ function collect_scheduled_activities(sim) return tuple(writers..., callbacks...) end -function schedule_aligned_Δt(sim, aligned_Δt) +function schedule_aligned_time_step(sim, aligned_Δt) clock = sim.model.clock activities = collect_scheduled_activities(sim) for activity in activities - aligned_Δt = aligned_time_step(activity.schedule, clock, aligned_Δt) + aligned_Δt = schedule_aligned_time_step(activity.schedule, clock, aligned_Δt) end return aligned_Δt @@ -44,7 +44,7 @@ function aligned_time_step(sim::Simulation, Δt) aligned_Δt = Δt # Align time step with output writing and callback execution - aligned_Δt = schedule_aligned_Δt(sim, aligned_Δt) + aligned_Δt = schedule_aligned_time_step(sim, aligned_Δt) # Align time step with simulation stop time aligned_Δt = min(aligned_Δt, unit_time(sim.stop_time - clock.time)) diff --git a/src/Utils/schedules.jl b/src/Utils/schedules.jl index bf710a835a..941ae2763d 100644 --- a/src/Utils/schedules.jl +++ b/src/Utils/schedules.jl @@ -10,7 +10,7 @@ false. abstract type AbstractSchedule end # Default behavior is no alignment. -aligned_time_step(schedule, clock, Δt) = Δt +schedule_aligned_time_step(schedule, clock, Δt) = Δt # Fallback initialization for schedule: call the schedule, # then return `true`, indicating that the schedule "actuates" at @@ -35,7 +35,8 @@ according to `model.clock.time`. """ mutable struct TimeInterval <: AbstractSchedule interval :: Float64 - previous_actuation_time :: Float64 + start_time :: Float64 + actuations :: Int end """ @@ -44,26 +45,37 @@ end Return a callable `TimeInterval` that schedules periodic output or diagnostic evaluation on a `interval` of simulation time, as kept by `model.clock`. """ -TimeInterval(interval) = TimeInterval(Float64(interval), 0.0) +TimeInterval(interval) = TimeInterval(convert(Float64, interval), 0.0, 0) + +function initialize!(schedule::TimeInterval, model) + schedule.start_time = time(model) + schedule(model) + return true +end + +function next_actuation_time(schedule::TimeInterval) + t₀ = schedule.start_time + N = schedule.actuations + T = schedule.interval + return = t₀ + N * T +end function (schedule::TimeInterval)(model) - time = model.clock.time + t = model.clock.time + t★ = next_actuation_time(schedule) - if time == schedule.previous_actuation_time + schedule.interval - schedule.previous_actuation_time = time - return true - elseif time > schedule.previous_actuation_time + schedule.interval - # Shave overshoot off previous_actuation_time to prevent overshoot from accumulating - schedule.previous_actuation_time = time - rem(time, schedule.interval) + if t >= t★ + schedule.actuations += 1 return true else return false end end -function aligned_time_step(schedule::TimeInterval, clock, Δt) - next_actuation_time = schedule.previous_actuation_time + schedule.interval - return min(Δt, next_actuation_time - clock.time) +function schedule_aligned_time_step(schedule::TimeInterval, clock, Δt) + t★ = next_actuation_time(schedule) + t = clock.time + return min(Δt, t★ - t) end ##### @@ -149,7 +161,7 @@ whenever the model's clock equals the specified values in `times`. For example, SpecifiedTimes(times::Vararg{T}) where T<:Number = SpecifiedTimes(sort([Float64(t) for t in times]), 0) SpecifiedTimes(times) = SpecifiedTimes(times...) -function next_appointment_time(st::SpecifiedTimes) +function next_actuation_time(st::SpecifiedTimes) if st.previous_actuation >= length(st.times) return Inf else @@ -160,7 +172,7 @@ end function (st::SpecifiedTimes)(model) current_time = model.clock.time - if current_time >= next_appointment_time(st) + if current_time >= next_actuation_time(st) st.previous_actuation += 1 return true end @@ -170,7 +182,7 @@ end initialize!(st::SpecifiedTimes, model) = st(model) -align_time_step(schedule::SpecifiedTimes, clock, Δt) = min(Δt, next_appointment_time(schedule) - clock.time) +align_time_step(schedule::SpecifiedTimes, clock, Δt) = min(Δt, next_actuation_time(schedule) - clock.time) function specified_times_str(st) str_elems = ["$(prettytime(t)), " for t in st.times] @@ -214,8 +226,8 @@ function (schedule::ConsecutiveIterations)(model) end end -aligned_time_step(schedule::ConsecutiveIterations, clock, Δt) = - aligned_time_step(schedule.parent, clock, Δt) +schedule_aligned_time_step(schedule::ConsecutiveIterations, clock, Δt) = + schedule_aligned_time_step(schedule.parent, clock, Δt) ##### ##### Any and AndSchedule From 2a28af30982f48cc692231a31d45bc0a27e4ae0c Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 14:09:33 +0200 Subject: [PATCH 2/9] fix typo --- src/Utils/schedules.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Utils/schedules.jl b/src/Utils/schedules.jl index 941ae2763d..1e14ee19fd 100644 --- a/src/Utils/schedules.jl +++ b/src/Utils/schedules.jl @@ -35,7 +35,7 @@ according to `model.clock.time`. """ mutable struct TimeInterval <: AbstractSchedule interval :: Float64 - start_time :: Float64 + first_actuation_time :: Float64 actuations :: Int end @@ -48,16 +48,16 @@ on a `interval` of simulation time, as kept by `model.clock`. TimeInterval(interval) = TimeInterval(convert(Float64, interval), 0.0, 0) function initialize!(schedule::TimeInterval, model) - schedule.start_time = time(model) + schedule.first_actuation_time = time(model) schedule(model) return true end function next_actuation_time(schedule::TimeInterval) - t₀ = schedule.start_time + t₀ = schedule.first_actuation_time N = schedule.actuations T = schedule.interval - return = t₀ + N * T + return t₀ + N * T end function (schedule::TimeInterval)(model) From 4642bfa8e4063ec549a654815aedd04087260599 Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 14:10:52 +0200 Subject: [PATCH 3/9] Import fix --- src/Simulations/run.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Simulations/run.jl b/src/Simulations/run.jl index 591d249f59..b29931b536 100644 --- a/src/Simulations/run.jl +++ b/src/Simulations/run.jl @@ -7,7 +7,7 @@ using Oceananigans: AbstractModel, run_diagnostic!, write_output! import Oceananigans: initialize! import Oceananigans.OutputWriters: checkpoint_path, set! import Oceananigans.TimeSteppers: time_step! -import Oceananigans.Utils: aligned_time_step +import Oceananigans.Utils: schedule_aligned_time_step # Simulations are for running From 7e099ee6e858a2af6a0436876d108a90e682f2f5 Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 17:26:45 +0200 Subject: [PATCH 4/9] Potentially address overflow problem --- src/Utils/schedules.jl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Utils/schedules.jl b/src/Utils/schedules.jl index 1e14ee19fd..15512f8a10 100644 --- a/src/Utils/schedules.jl +++ b/src/Utils/schedules.jl @@ -65,7 +65,12 @@ function (schedule::TimeInterval)(model) t★ = next_actuation_time(schedule) if t >= t★ - schedule.actuations += 1 + if schedule.actuations < typemax(Int) + schedule.actuations += 1 + else + schedule.first_actuation_time = t★ + schedule.actuations = 1 + end return true else return false From 8b8a4267e3b49a8a69502e23a3ef865238fe0e28 Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 18:43:06 +0200 Subject: [PATCH 5/9] Recompute new third stage time-step to reduce error accumulation --- src/TimeSteppers/runge_kutta_3.jl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/TimeSteppers/runge_kutta_3.jl b/src/TimeSteppers/runge_kutta_3.jl index 9c706c88ff..ba53200335 100644 --- a/src/TimeSteppers/runge_kutta_3.jl +++ b/src/TimeSteppers/runge_kutta_3.jl @@ -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 # @@ -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) From e76934d019ed53cc92d7839aa9ab85fa5a7bec16 Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 19:58:13 +0200 Subject: [PATCH 6/9] Make schedule work even with fake models --- src/Utils/schedules.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils/schedules.jl b/src/Utils/schedules.jl index 15512f8a10..f13749bc59 100644 --- a/src/Utils/schedules.jl +++ b/src/Utils/schedules.jl @@ -48,7 +48,7 @@ on a `interval` of simulation time, as kept by `model.clock`. TimeInterval(interval) = TimeInterval(convert(Float64, interval), 0.0, 0) function initialize!(schedule::TimeInterval, model) - schedule.first_actuation_time = time(model) + schedule.first_actuation_time = model.clock.time schedule(model) return true end From e5dbae3a0e63d7e7a1a873e35c4a4e37a757dbea Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 21:37:05 +0200 Subject: [PATCH 7/9] Fix tests for TimeInterval --- test/test_schedules.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_schedules.jl b/test/test_schedules.jl index a105364e7c..5b49c017cb 100644 --- a/test/test_schedules.jl +++ b/test/test_schedules.jl @@ -19,6 +19,9 @@ using Oceananigans: initialize! # TimeInterval ti = TimeInterval(2) + initialize!(ti, fake_model_at_iter_0) + + @test ti.actuations == 1 @test ti.interval == 2.0 @test ti(fake_model_at_time_2) @test !(ti(fake_model_at_time_3)) From bf2298daf5e0c01aa7cf684e7c589b40b520b35d Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 21:45:35 +0200 Subject: [PATCH 8/9] Fix JLD2 output writer tests --- src/OutputWriters/OutputWriters.jl | 2 +- src/OutputWriters/jld2_output_writer.jl | 1 + src/OutputWriters/output_writer_utils.jl | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/OutputWriters/OutputWriters.jl b/src/OutputWriters/OutputWriters.jl index 0f66cee682..d9cbe64953 100644 --- a/src/OutputWriters/OutputWriters.jl +++ b/src/OutputWriters/OutputWriters.jl @@ -19,7 +19,7 @@ using Oceananigans.Utils: pretty_filesize using OffsetArrays -import Oceananigans: write_output! +import Oceananigans: write_output!, initialize! Base.open(ow::AbstractOutputWriter) = nothing Base.close(ow::AbstractOutputWriter) = nothing diff --git a/src/OutputWriters/jld2_output_writer.jl b/src/OutputWriters/jld2_output_writer.jl index b97ccbf2b4..9034f8db50 100644 --- a/src/OutputWriters/jld2_output_writer.jl +++ b/src/OutputWriters/jld2_output_writer.jl @@ -178,6 +178,7 @@ function JLD2OutputWriter(model, outputs; filename, schedule, mkpath(dir) filename = auto_extension(filename, ".jld2") filepath = joinpath(dir, filename) + initialize!(file_splitting, model) update_file_splitting_schedule!(file_splitting, filepath) overwrite_existing && isfile(filepath) && rm(filepath, force=true) diff --git a/src/OutputWriters/output_writer_utils.jl b/src/OutputWriters/output_writer_utils.jl index 9218e44d58..30e3cead20 100644 --- a/src/OutputWriters/output_writer_utils.jl +++ b/src/OutputWriters/output_writer_utils.jl @@ -16,6 +16,7 @@ struct NoFileSplitting end (::NoFileSplitting)(model) = false Base.summary(::NoFileSplitting) = "NoFileSplitting" Base.show(io::IO, nfs::NoFileSplitting) = print(io, summary(nfs)) +initialize!(::NoFileSplitting, model) = nothing mutable struct FileSizeLimit <: AbstractSchedule size_limit :: Float64 @@ -32,7 +33,6 @@ The `path` is automatically added and updated when `FileSizeLimit` is used with an output writer, and should not be provided manually. """ FileSizeLimit(size_limit) = FileSizeLimit(size_limit, "") - (fsl::FileSizeLimit)(model) = filesize(fsl.path) ≥ fsl.size_limit function Base.summary(fsl::FileSizeLimit) From a7628a4553b1505e01cad27ae4498936cf298b30 Mon Sep 17 00:00:00 2001 From: Gregory Wagner Date: Thu, 6 Jun 2024 21:46:43 +0200 Subject: [PATCH 9/9] Fix NetCDF writer test? --- src/OutputWriters/netcdf_output_writer.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OutputWriters/netcdf_output_writer.jl b/src/OutputWriters/netcdf_output_writer.jl index 56d328296b..6b83fd6e09 100644 --- a/src/OutputWriters/netcdf_output_writer.jl +++ b/src/OutputWriters/netcdf_output_writer.jl @@ -414,6 +414,7 @@ function NetCDFOutputWriter(model, outputs; filename = auto_extension(filename, ".nc") filepath = joinpath(dir, filename) + initialize!(file_splitting, model) update_file_splitting_schedule!(file_splitting, filepath) if isnothing(overwrite_existing)