Callback with SpecifiedTimes also gets called in iteration 0

[tomchor]

Right now SpecifiedTimes is acting in a way different from its documentation. In addition to triggering a callback in the specified times, according to the docs, it's also triggering the callback in initialization.

For example, the following MWE

using Oceananigans

grid = RectilinearGrid(size=(4, 4, 4), extent=(1, 1, 1))
model = NonhydrostaticModel(; grid)
simulation = Simulation(model, Δt=1, stop_iteration=10)

callback_func(sim) = @warn "Function called in iteration $(sim.model.clock.iteration)"
simulation.callbacks[:cfl_changer] = Callback(callback_func, SpecifiedTimes([5, 10]))

run!(simulation)

produces the following output

[ Info: Initializing simulation...
┌ Warning: Function called in iteration 0
└ @ Main ~/repos/Oceananigans.jl/sandbox/test_specified_times.jl:7
[ Info:     ... simulation initialization complete (262.483 ms)
[ Info: Executing initial time step...
[ Info:     ... initial time step complete (28.766 seconds).
┌ Warning: Function called in iteration 5
└ @ Main ~/repos/Oceananigans.jl/sandbox/test_specified_times.jl:7
[ Info: Simulation is stopping. Model iteration 10 has hit or exceeded simulation stop iteration 10.
┌ Warning: Function called in iteration 10
└ @ Main ~/repos/Oceananigans.jl/sandbox/test_specified_times.jl:7

Is this by design? If so the docs for SpecifiedTimes must be changed to account for that since they currently read:

  Return a callable TimeInterval that "actuates" (schedules output or callback execution) whenever the model's clock equals the specified values in times. For example,

    •  SpecifiedTimes([1, 15.3]) actuates when model.clock.time is 1 and 15.3.

[glwagner]

All of the callbacks are executed on iteration 0 currently:

Oceananigans.jl/src/Simulations/run.jl

Lines 157 to 205 in af21542

	"""
	initialize_simulation!(sim, pickup=false)
	Initialize a simulation:
	- Update the auxiliary state of the simulation (filling halo regions, computing auxiliary fields)
	- Evaluate all diagnostics, callbacks, and output writers if sim.model.clock.iteration == 0
	- Add diagnostics that "depend" on output writers
	"""
	function initialize_simulation!(sim)
	@info "Initializing simulation..."
	start_time = time_ns()
	model = sim.model
	clock = model.clock
	update_state!(model)
	# Output and diagnostics initialization
	[add_dependencies!(sim.diagnostics, writer) for writer in values(sim.output_writers)]
	# Reset! the model time-stepper, evaluate all diagnostics, and write all output at first iteration
	if clock.iteration == 0
	reset!(sim.model.timestepper)
	# Initialize schedules and run diagnostics, callbacks, and output writers
	for diag in values(sim.diagnostics)
	diag.schedule(sim.model)
	run_diagnostic!(diag, model)
	end
	for callback in values(sim.callbacks)
	callback.schedule(model)
	callback(sim)
	end
	for writer in values(sim.output_writers)
	writer.schedule(sim.model)
	write_output!(writer, model)
	end
	end
	sim.initialized = true
	initialization_time = prettytime(1e-9 * (time_ns() - start_time))
	@info " ... simulation initialization complete ($initialization_time)"
	return nothing
	end

[glwagner]

I'm not sure what's best. We're assuming that iteration 0 is scheduled. I guess under ordinary circumstances, iteration 0 is scheduled for IterationInterval and TimeInterval --- though this need not always be true. Also we might argue that iteration 0 should not be scheduled automatically for SpecifiedTimes.

Perhaps schedules themselves should somehow explicitly specify whether they should be actuated at iteration 0 or not.

A downside of avoiding iteration 0 is that issues / bugs with a callback are not caught until first actuation. So it may be a sensible default to actuate at iteration 0.

[tomchor]

I would argue that SpecifiedTimes shouldn't be actuated in iteration 0 since, if that is desired by the user, it's trivially easy to add zero to the list of times. I also think it's counter-intuitive to have something with SpecifiedTimes([5, 10]) to actuate at t=[0, 5, 10] in practice, although I guess that's up for debate.

If no one opposes this, I can open a PR to make this change.

[glwagner]

This is about more than just defining inuitive behavior, though. Executing a callback at iteration 0 might be considered a feature. However, I think that sometimes it's not desired. In reality, what we are missing is the concept of callback "initialization" (we are also missing the concept of callback "finalization"). Right now, we use the hack that "calling at iteration 0" is tantamount to initialization. I think we should discuss how to generalize our design to something more sustainable...

[navidcy]

I think we should call them at iteration 0.

Ideally we want an initialization and finalization; that would be useful. There are things one needs to do when all party is done (i.e., finalization).

[glwagner]

What if we add this feature to Callback? I.e.

struct Callback{P, F, S, I}
    func :: F
    schedule :: S
    parameters :: P
    initialize :: I
end

Then by default we set

Callback(; ..., initialize=call_at_iteration_0)

where

call_at_iteration_0(callback, simulation) = iteration(simulation) == 0 && callback(simulation)

so the default "initialization" is simply to "call" the callback at iteration 0 (as we currently do). Users can cancel this by setting initialize=nothing or providing some alternative function.

Finally, rather than calling all the callbacks at iteration 0, we instead call Callback.initialize! for every callback inside initalize_simulation!:

Oceananigans.jl/src/Simulations/run.jl

Line 166 in af21542

function initialize_simulation!(sim)

For "finalization" we need a bit more work, since I think we want to add the concept of finalizing a simulation as well, so we might need Simulation.finalized. That's probably a nice idea too though.

[navidcy]

Yes I like that. And I like the idea of Simulation.finalized.

Note that a feature like that comes in needed in, e.g., #2657.

[glwagner]

I can work on this. We can build

function finalize!(sim::Simulation)
    # Finalize callbacks
    [cb.finalize(cb, sim) for cb in sim.callbacks]

    # Finalize model
    finalize!(sim.model)

    return nothing
end

Models can then define appropriate finalizations. One layer down, we can have

function finalize!(model::HydrostaticFreeSurfaceModel)
    finalize!(model.free_surface)
    return nothing
end

[navidcy]

I can work on this. We can build

function finalize!(sim::Simulation)
    # Finalize callbacks
    [cb.finalize(cb, sim) for cb in sim.callbacks]

    # Finalize model
    finalize!(sim.model)

    return nothing
end

Models can then define appropriate finalizations. One layer down, we can have

function finalize!(model::HydrostaticFreeSurfaceModel)
    finalize!(model.free_surface)
    return nothing
end

Let's do this! :)