[WIP] Implement FastRK3 from Aithal and Ferrante (2020) for nonhydrostatic models#5059
[WIP] Implement FastRK3 from Aithal and Ferrante (2020) for nonhydrostatic models#5059
FastRK3 from Aithal and Ferrante (2020) for nonhydrostatic models#5059Conversation
| pⁿ :: TP # Current pressure (∇φⁿ) | ||
| pⁿ⁻¹ :: TP # Previous pressure (∇φⁿ⁻¹) | ||
| Δt⁻¹ :: FT # Previous time step | ||
| RK3 :: RungeKutta3TimeStepper{FT, TG, TI} # For initialization |
There was a problem hiding this comment.
can we avoid storing this? it will double the storage
There was a problem hiding this comment.
you can do a few things instead:
- you can figure out how to call the RK3 time-step methods avoiding the dispatch issue
- you can build
RungeKuttaTimeStepperusing the fields ofFastRungeKutta3TimeStepper. this is possible because the latter has the two tendencies that you need, plus the implicit solver.
There was a problem hiding this comment.
I now see that you indeed re-use the tendency fields. I guess it's either/or... my hunch is that it is better to build the new timestepper on the fly rather than store this reference.
| G⁰ :: TG # Storage for tendencies at stage 0 | ||
| G⁻ :: TG # Storage for previous tendencies | ||
| Gⁿ :: TG # Current stage tendencies |
There was a problem hiding this comment.
does it really require 3 copies of the tendencies instead of 2? so the storage requirement is significantly more?
There was a problem hiding this comment.
I think it does. But there might be a clever way or a different RK3 scheme that might avoid this issue. I'll have to think about it more, it's currently an untested first draft, I'll have to check the coefficients and my derivation more closely
There was a problem hiding this comment.
I also don't see why we can't use our current RK3 scheme and just try a pressure extrapolation, it might even work out
There was a problem hiding this comment.
We can get away with only one additional pressure field storage rather than 2 I have currently implemented, since we only need two time points for a linear extrapolation. We could get away with only two tendency storage if we apply this technique to the currently RK3 implementation
There was a problem hiding this comment.
I think it would be valid to implement this in the existing RungeKutta3TimeStepper. it is always nice to extend implementations, rather than create duplicates with lots of overlapping code (wink, hint, wink). You can add an additiona property representing the previous pressure field. When that previous pressure field is nothing, we do RK3 as usual. When we have the property available, we use the "Fast" version. This is similar to how we are able to compute/ not compute the hydrostatic pressure (without creating an entirely new NonhydrostaticModel just for that)
| Gⁿ :: TG # Current stage tendencies | ||
| pⁿ :: TP # Current pressure (∇φⁿ) | ||
| pⁿ⁻¹ :: TP # Previous pressure (∇φⁿ⁻¹) | ||
| Δt⁻¹ :: FT # Previous time step |
There was a problem hiding this comment.
the previous time-step is stored in clock. do you also need it here? it is typically better not to store redundant inforation, because you can experience bugs when the information is inconsistent (eg "single source of truth" concept)
|
|
||
| # If needs initialization, zero out the pressure gradient storage | ||
| if needs_initialization | ||
| time_step!(model, model.timestepper.RK3, Δt; callbacks=callbacks) |
There was a problem hiding this comment.
consider doing something like
vanilla_rk3_timestepper = RungeKutta3TimeStepper(...)
time_step!(model, vanilla_rk3_timestepper, Δt; callbacks=callbacks)2 participants
This aims to close #4804. By trading for one more tendency storage and two more pressure field storage, it is perhaps possible to compute one Poisson solve per time step, which would speed things up by roughly 3x.