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.

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

Clock keeps ticking if we insist running the simulation beyond a stop criterion #4253

Open
navidcy opened this issue Mar 20, 2025 · 6 comments
Labels
question 💭 No such thing as a stupid question

Comments

@navidcy
Copy link
Collaborator

navidcy commented Mar 20, 2025

I demonstrate what I mean below:

julia> using Oceananigans

julia> grid = RectilinearGrid(size=(4, 3, 2), extent=(2, 3, 4));

julia> model = HydrostaticFreeSurfaceModel(; grid);

julia> simulation = Simulation(model, Δt=1, stop_iteration = 3);

julia> run!(simulation)
[ Info: Initializing simulation...
[ Info:     ... simulation initialization complete (931.391 ms)
[ Info: Executing initial time step...
[ Info:     ... initial time step complete (4.934 seconds).
[ Info: Simulation is stopping after running for 0 seconds.
[ Info: Model iteration 3 equals or exceeds stop iteration 3.

julia> run!(simulation)
[ Info: Initializing simulation...
[ Info:     ... simulation initialization complete (1.780 ms)
[ Info: Executing initial time step...
[ Info: Simulation is stopping after running for 0 seconds.
[ Info: Model iteration 4 equals or exceeds stop iteration 3.
[ Info:     ... initial time step complete (215.624 ms).

julia> run!(simulation)
[ Info: Initializing simulation...
[ Info:     ... simulation initialization complete (1.539 ms)
[ Info: Executing initial time step...
[ Info: Simulation is stopping after running for 0 seconds.
[ Info: Model iteration 5 equals or exceeds stop iteration 3.
[ Info:     ... initial time step complete (209.062 ms).

julia> run!(simulation)
[ Info: Initializing simulation...
[ Info:     ... simulation initialization complete (2.555 ms)
[ Info: Executing initial time step...
[ Info: Simulation is stopping after running for 0 seconds.
[ Info: Model iteration 6 equals or exceeds stop iteration 3.
[ Info:     ... initial time step complete (3.754 ms).

Should we be ticking the clock at the end of the timestep?

@navidcy navidcy added the question 💭 No such thing as a stupid question label Mar 20, 2025
@navidcy
Copy link
Collaborator Author

navidcy commented Mar 20, 2025

Or check the simulation perhaps should check the stop_criteria before it ticks the clock?

@glwagner
Copy link
Member

I think that's because I tried to simplify time_step! here:

6c254b8

I thought it was ok that we always take one time step.

But we can reinstate that check ! We used to throw an error of the nature "simulation stopped during initialization".

Or maybe there is a better way? We can move the check into run!.

@glwagner
Copy link
Member

A related question, which is really just a definition, is whether callbacks should always be executed at the beginning of run!, or only when iteration == 0. We do the latter right now:

if model.clock.iteration == 0
reset!(timestepper(model))
# Initialize schedules and run diagnostics, callbacks, and output writers
for diag in values(sim.diagnostics)
run_diagnostic!(diag, model)
end
for callback in values(sim.callbacks)
callback.callsite isa TimeStepCallsite && callback(sim)
end
for writer in values(sim.output_writers)
writer.schedule(model)
write_output!(writer, model)
end
end

Another idea is to make "stop_criteria" into a special kind of callback that has different behavior than the other callbacks. Presumably there's no harm in running stop criteria as much needed (vs other callbacks / output writers, which if called more than desired can do things like mess up your time series of data).

@glwagner
Copy link
Member

Another simplification would be to combine diagnostics, output writers, and callbacks into a single dict, and just label them all as "callbacks".

@glwagner
Copy link
Member

glwagner commented Mar 20, 2025

Actually I am confused whether the change I noted actually caused this. Anyways, note this line used to present, but is no longer there.

@warn "Simulation stopped during initialization."

I have a new idea.

What if we define initialize! for callbacks and output writers differently:

  • for callbacks, we call initialize! on callback.func, and then execute the callback.
  • for output writers and diagnostics, we write output only if iteration == 0, preserving existing behavior. Right now I don't think we utilize the initialize! method for output writers.

The advantage of this approach, instead of hard-coding the dependency on iteration==0 is this:

  1. Because callbacks are always called at initialization by default, they have the ability to stop a simulation before it starts.
  2. Users can override this definition by providing a more specific initialize! for their callback, so this default behavior is not written in stone.

Next, we place a call to initialize!(sim) in run!. We do want to keep the call in time_step! too, but this will no longer be called if using run! (it would only be called if using time_step!(sim) directly --- like ClimaOcean does).

So now, sim.running can be set to false during initialization. Because run! looks like this:

    sim.initialized = false
    sim.running = true
    sim.run_wall_time = 0.0
    initialize!(sim)

    while sim.running
        time_step!(sim)
    end

then time_step! will not be called if the clock has already past its stop time / iteration.

@glwagner
Copy link
Member

I can write this up if you agree @navidcy .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question 💭 No such thing as a stupid question
Projects
None yet
Development

No branches or pull requests

2 participants