Oceananigans.jl
Oceananigans.jl copied to clipboard
(0.96.2) Clock and Checkpointer updates
This PR enhances tick!(clock, Δt). Also defines method for set_clock!(clock, new_clock).
Historically this PR started by implementing some changes needed for the implementation of a Checkpointer for the OceanSeaIceModel at https://github.com/CliMA/ClimaOcean.jl/pull/401. But in the process we bumped onto things related to clock and time stepping.
Main change is that properties is no longer a field of the Checkpointer but rather it's inferred from the model. This way we have different properties for the different model components of a coupled model in ClimaOcean.
Also the PR adds tests for the Checkpointer that use the SplitExplicitFreeSurface (two formulations via constant substeps and given cfl).
@glwagner the CI machine seems out of space?
ERROR: Unable to automatically download/install artifact 'CUDA_Runtime' from sources listed in '/storage5/buildkite-agent/.julia-21600/packages/CUDA_Runtime_jll/iU7vK/Artifacts.toml'.
Sources attempted:
- https://pkg.julialang.org/artifact/59c3479d656c293c66e1a7a26fcb5d860dbe6235
Error: SystemError: flush: No space left on device
- https://github.com/JuliaBinaryWrappers/CUDA_Runtime_jll.jl/releases/download/CUDA_Runtime-v0.16.1+0/CUDA_Runtime.v0.16.1.x86_64-linux-gnu-cuda+12.8.tar.gz
Error: SystemError: flush: No space left on device
Do the changes here have any impact on https://github.com/CliMA/Oceananigans.jl/issues/3485?
i.e. will it make it easier or harder to make Checkpointer play with time-averaged fields?
We should probably at the least address the problem of model vs simulation checkpointing here
@tomchor it shouldn't affect the time-averaging issue in either way I think.
@glwagner, @tomchor so I see in https://github.com/CliMA/Oceananigans.jl/issues/3485 the discussion/proposal is to have write_output!(sim::Simulation, ...) instead of write_output!(model::AbstractModel, ...).
We can do that here. But then it means that every time somebody defines a new model (and I'm thinking of ClimaOcean.jl and beyond) then we need to define a new method for write_output!(sim::Simulation{MyNewModel}, ...) instead of write_output!(model::MyNewModel, ...), right?
@glwagner, @tomchor so I see in #3485 the discussion/proposal is to have
write_output!(sim::Simulation, ...)instead ofwrite_output!(model::AbstractModel, ...).We can do that here. But then it means that every time somebody defines a new model (and I'm thinking of ClimaOcean.jl and beyond) then we need to define a new method for
write_output!(sim::Simulation{MyNewModel}, ...)instead ofwrite_output!(model::MyNewModel, ...), right?
I think so, but wouldn't a fallback like write_output!(simulation) = write_output!(simulation.model) take care of this?
@glwagner, @tomchor so I see in #3485 the discussion/proposal is to have
write_output!(sim::Simulation, ...)instead ofwrite_output!(model::AbstractModel, ...). We can do that here. But then it means that every time somebody defines a new model (and I'm thinking of ClimaOcean.jl and beyond) then we need to define a new method forwrite_output!(sim::Simulation{MyNewModel}, ...)instead ofwrite_output!(model::MyNewModel, ...), right?I think so, but wouldn't a fallback like
write_output!(simulation) = write_output!(simulation.model)take care of this?
Yeah no I think we can have a pretty small PR that simply changes the meaning of write_output!. A few lines are all that's needed:
write_output!(writer, sim::Simulation) = write_output!(writer, sim.model)
and then I think 2-3 places in Simulations where we call write_output!.
I'd prefer if that was self-contained, we should avoid the temptation to make PRs bigger than needed.
The trickier part (but not the hardest thing ever in the world) is to design an interface for checkpointing callbacks, diagnostics, and output writers.
This PR includes few useful updates/refactors for Clock and I'm quite tempted to remove the Checkpointer things, merge and continue with Checkpointer in a new PR. What do you think?
This PR includes few useful updates/refactors for Clock and I'm quite tempted to remove the Checkpointer things, merge and continue with Checkpointer in a new PR. What do you think?
I'm all for simple PRs!
https://github.com/CliMA/Oceananigans.jl/pull/4637 attempts to separate the Clock enhancements