Oceananigans.jl icon indicating copy to clipboard operation
Oceananigans.jl copied to clipboard

(0.96.2) Clock and Checkpointer updates

Open navidcy opened this issue 8 months ago • 7 comments

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).

navidcy avatar Mar 20 '25 04:03 navidcy

@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

navidcy avatar Mar 20 '25 11:03 navidcy

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?

tomchor avatar Mar 22 '25 18:03 tomchor

We should probably at the least address the problem of model vs simulation checkpointing here

glwagner avatar Mar 22 '25 18:03 glwagner

@tomchor it shouldn't affect the time-averaging issue in either way I think.

navidcy avatar Mar 22 '25 22:03 navidcy

@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?

navidcy avatar Mar 22 '25 22:03 navidcy

@glwagner, @tomchor so I see in #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?

I think so, but wouldn't a fallback like write_output!(simulation) = write_output!(simulation.model) take care of this?

tomchor avatar Mar 25 '25 01:03 tomchor

@glwagner, @tomchor so I see in #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?

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.

glwagner avatar Mar 25 '25 03:03 glwagner

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?

navidcy avatar Jun 26 '25 00:06 navidcy

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!

tomchor avatar Jun 26 '25 01:06 tomchor

https://github.com/CliMA/Oceananigans.jl/pull/4637 attempts to separate the Clock enhancements

navidcy avatar Jul 06 '25 09:07 navidcy