Can we add last_Δt and last_stage_Δt to tick!

[jagoosw]

Is there any reason we can't add the last_Δt and last_stage_Δt updates to tick! since its a bit of a mess without?

Oceananigans.jl/src/TimeSteppers/quasi_adams_bashforth_2.jl

Lines 106 to 107 in 7f2d512

	model.clock.last_Δt = Δt
	model.clock.last_stage_Δt = Δt # just one stage

Oceananigans.jl/src/TimeSteppers/runge_kutta_3.jl

Lines 125 to 126 in 7f2d512

	tick!(model.clock, second_stage_Δt; stage=true)
	model.clock.last_stage_Δt = second_stage_Δt

tick! could have kwargs final_stage which does the same as in the AB2 step if true and just updates the stage if not.

CC: @glwagner

[glwagner]

I felt that this pattern is more clear because we see immediately the meaning of the different storage variables without having to find and read the function tick! (whose ultimate purpose is to handle different time steps, even though we only use Float for now...)

[glwagner]

I guess the additional complication is that in order for this to work "automatically", then a multi-stage time-stepper also has to pass in the last_Δt in addition to last_stage_Δt during the final stage for this to work correctly.

I feel the issue is that the code gets sort of convoluted. Whereas simply setting last_Δt correctly depending on the time-stepper being used seems fairly straightforward.

This is what it has to look like I think, or something like this...

function tick!(clock, Δt, last_Δt = Δt; intermediate_stage=false)

    tick_time!(clock, Δt)

    if intermediate_stage # tick a stage update
        clock.stage += 1
        clock.last_stage_Δt = Δt 
    else # tick an iteration and reset stage
        clock.iteration += 1
        clock.stage = 1
        clock.last_stage_Δt = Δt 
        clock.last_Δt = last_Δt 
    end

    return nothing
end

Then for AB2 we just call tick!(clock, Δt). For RK3 we have to call tick!(clock, stage_Δt, intermediate_stage=true) twice, and then finally tick!(clock, stage_Δt, Δt) after the third stage.

[glwagner]

It's tempting to make systems for things but we only have two cases and I think it's easier to explicitly write what we are doing. So, I am against this change.

[jagoosw]

yeah I agree that is more complicated that its worth since there is only the two timesteppers anyway