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

Jump to bottom

Adds last_Δt to Clock #3508

Merged
merged 19 commits into from
Mar 21, 2024
Merged

Adds last_Δt to Clock #3508

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ac59cdf
added `Δt` to `Clock`
jagoosw Mar 13, 2024
dfd62df
fixed clock to work for DateTime
jagoosw Mar 13, 2024
99aacbb
changed name to `last_Δt`
jagoosw Mar 13, 2024
a68c34a
fixed Clock adapt
jagoosw Mar 13, 2024
51f792b
updated docstring
jagoosw Mar 13, 2024
1852d42
Update src/TimeSteppers/clock.jl
jagoosw Mar 13, 2024
a83e3bb
fixed constructor
jagoosw Mar 13, 2024
40db431
updated `reset!`
jagoosw Mar 13, 2024
65068f6
fixed formatting
jagoosw Mar 13, 2024
d5f4916
Apply suggestions from code review
jagoosw Mar 14, 2024
bc3ca25
Update src/TimeSteppers/clock.jl
jagoosw Mar 18, 2024
689c56c
Merge branch 'main' into jsw/add-Δt-to-clock
glwagner Mar 18, 2024
0170cf0
increase size_threshold=1MiB
navidcy Mar 20, 2024
9820fed
fix doctest
navidcy Mar 20, 2024
28cc105
fix doctest
navidcy Mar 20, 2024
b7d3253
Update pipeline.yml
navidcy Mar 20, 2024
a48cbb8
Merge branch 'main' into jsw/add-Δt-to-clock
navidcy Mar 20, 2024
84c206b
fix size_threshold
navidcy Mar 21, 2024
5af5f82
Merge branch 'jsw/add-Δt-to-clock' of github.com:CliMA/Oceananigans.j…
navidcy Mar 21, 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
4 changes: 2 additions & 2 deletions src/Models/HydrostaticFreeSurfaceModels/hydrostatic_free_surface_model.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ end

"""
HydrostaticFreeSurfaceModel(; grid,
clock = Clock{eltype(grid)}(0, 0, 1),
clock = Clock{eltype(grid), eltype(grid)}(0, Inf, 0, 1),
momentum_advection = CenteredSecondOrder(),
tracer_advection = CenteredSecondOrder(),
buoyancy = SeawaterBuoyancy(eltype(grid)),
Expand Down Expand Up @@ -94,7 +94,7 @@ Keyword arguments
- `auxiliary_fields`: `NamedTuple` of auxiliary fields. Default: `nothing`.
"""
function HydrostaticFreeSurfaceModel(; grid,
clock = Clock{eltype(grid)}(0, 0, 1),
clock = Clock{eltype(grid), eltype(grid)}(0, Inf, 0, 1),
momentum_advection = CenteredSecondOrder(),
tracer_advection = CenteredSecondOrder(),
buoyancy = SeawaterBuoyancy(eltype(grid)),
Expand Down
4 changes: 2 additions & 2 deletions src/Models/NonhydrostaticModels/nonhydrostatic_model.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ end

"""
NonhydrostaticModel(; grid,
clock = Clock{eltype(grid)}(0, 0, 1),
clock = Clock{eltype(grid), eltype(grid)}(0, Inf, 0, 1),
advection = CenteredSecondOrder(),
buoyancy = nothing,
coriolis = nothing,
Expand Down Expand Up @@ -105,7 +105,7 @@ Keyword arguments
- `auxiliary_fields`: `NamedTuple` of auxiliary fields. Default: `nothing`
"""
function NonhydrostaticModel(; grid,
clock = Clock{eltype(grid)}(0, 0, 1),
clock = Clock{eltype(grid), eltype(grid)}(0, Inf, 0, 1),
advection = CenteredSecondOrder(),
buoyancy = nothing,
coriolis = nothing,
Expand Down
4 changes: 2 additions & 2 deletions src/Models/ShallowWaterModels/shallow_water_model.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ struct VectorInvariantFormulation end
"""
ShallowWaterModel(; grid,
gravitational_acceleration,
clock = Clock{eltype(grid)}(0, 0, 1),
clock = Clock{eltype(grid), eltype(grid)}(0, Inf, 0, 1),
momentum_advection = UpwindBiasedFifthOrder(),
tracer_advection = WENO(),
mass_advection = WENO(),
Expand Down Expand Up @@ -112,7 +112,7 @@ Keyword arguments
function ShallowWaterModel(;
grid,
gravitational_acceleration,
clock = Clock{eltype(grid)}(0, 0, 1),
clock = Clock{eltype(grid), eltype(grid)}(0, Inf, 0, 1),
Copy link
Member

Choose a reason for hiding this comment

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

Personally I would add a constructor for Clock that takes one type rather than writing this. But you can decide.

Copy link
Member

@glwagner glwagner Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also note that we might reasonably expect users to provide custom clocks with their own time and iteration --- but that the stage and time-step probably won't commonly be provided when building a clock. So I think it would make more sense to have a syntax that looks like

Clock{TT}(time, iteration, [optional args])

Copy link
Member

@glwagner glwagner Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think the type of the time-step can probably be inferred from the time type TT (ie if the time type is either a floating point or a DateTime or something like that).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah that makes sense

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There actually is already a constructor I can just change it a bit to infer the DT

Oceananigans.jl/src/TimeSteppers/clock.jl

Line 27 in 3bb62a6

Clock(; time::T, iteration=0, stage=1) where T = Clock{T}(time, iteration, stage)

Copy link
Collaborator Author

@jagoosw jagoosw Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It seems like Dates only stores things as integers and is never directly used but the timestep is always eltype(grid) so I'm not sure it can be directly infered.

Copy link
Member

Choose a reason for hiding this comment

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

We don't support dates right now -- to support it, we have to generalize how the time-step specification works.

Copy link
Member

Choose a reason for hiding this comment

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

the timestep is always eltype(grid) so I'm not sure it can be directly infered.

We will have to change how time-step specification works to support DateTime properly.

momentum_advection = UpwindBiasedFifthOrder(),
tracer_advection = WENO(),
mass_advection = WENO(),
Expand Down
21 changes: 12 additions & 9 deletions src/TimeSteppers/clock.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,31 @@ import Base: show
import Oceananigans.Units: Time

"""
mutable struct Clock{T<:Number}
mutable struct Clock{T, FT}
navidcy marked this conversation as resolved.
Show resolved Hide resolved

Keeps track of the current `time`, `iteration` number, and time-stepping `stage`.
Keeps track of the current `time`, `last_Δt`, `iteration` number, and time-stepping `stage`.
The `stage` is updated only for multi-stage time-stepping methods. The `time::T` is
either a number or a `DateTime` object.
"""
mutable struct Clock{T}
mutable struct Clock{T, FT}
time :: T
last_Δt :: FT
jagoosw marked this conversation as resolved.
Show resolved Hide resolved
iteration :: Int
stage :: Int
end

"""
Clock(; time, iteration=0, stage=1)
Clock(; time, last_Δt = Inf, iteration=0, stage=1)

Returns a `Clock` object. By default, `Clock` is initialized to the zeroth `iteration`
and first time step `stage`.
and first time step `stage` with `last_Δt`.
"""
Clock(; time::T, iteration=0, stage=1) where T = Clock{T}(time, iteration, stage)
Clock(; time::T, last_Δt::FT = Inf, iteration=0, stage=1) where {T, FT} = Clock{T, FT}(time, last_Δt, iteration, stage)

Base.summary(clock::Clock) = string("Clock(time=$(prettytime(clock.time)), iteration=$(clock.iteration))")
Base.summary(clock::Clock) = string("Clock(time=$(prettytime(clock.time)), iteration=$(clock.iteration), last_Δt=$(prettytime(clock.last_Δt)))")

Base.show(io::IO, c::Clock{T}) where T =
println(io, "Clock{$T}: time = $(prettytime(c.time)), iteration = $(c.iteration), stage = $(c.stage)")
println(io, "Clock{$T}: time = $(prettytime(c.time)), last_Δt = $(prettytime(c.last_Δt)), iteration = $(c.iteration), stage = $(c.stage)")

next_time(clock, Δt) = clock.time + Δt
next_time(clock::Clock{<:AbstractTime}, Δt) = clock.time + Nanosecond(round(Int, 1e9 * Δt))
Expand All @@ -52,6 +53,8 @@ function tick!(clock, Δt; stage=false)

tick_time!(clock, Δt)

clock.last_Δt = Δt

if stage # tick a stage update
clock.stage += 1
else # tick an iteration and reset stage
Expand All @@ -63,4 +66,4 @@ function tick!(clock, Δt; stage=false)
end

"Adapt `Clock` to work on the GPU via CUDAnative and CUDAdrv."
Adapt.adapt_structure(to, clock::Clock) = (time=clock.time, iteration=clock.iteration, stage=clock.stage)
Adapt.adapt_structure(to, clock::Clock) = (time=clock.time, last_Δt=clock.last_Δt, iteration=clock.iteration, stage=clock.stage)
jagoosw marked this conversation as resolved.
Show resolved Hide resolved