Fix lock-exchange example#5248
Conversation
| conjure_time_step_wizard!(simulation, cfl=0.3) | ||
| # time-stepping with a Courant-Freidrichs-Lewy (CFL) number of 0.7. | ||
| # Note that the suitability of the wizard depends on the model's `timestepper`. | ||
| # In this case we use a single-step method (a Runge-Kutta discretization) |
There was a problem hiding this comment.
What does “single step” mean? It’s actually multi step, right?
There was a problem hiding this comment.
I think the convention is that RK is multi-stage but single-step (only one time-step is involved), while AB2 is single-stage multi-step (2 time-steps involved). However, we can come up with a better naming?
There was a problem hiding this comment.
I think the description should imply the timestepper has "memory" or not. When the timestepper has memory, changing the time-step requires re-starting the time-stepper. That's why it's better to use adaptive time-stepping wih a self-starting timestepper
| tracers = :b, | ||
| buoyancy = BuoyancyTracer(), | ||
| closure = CATKEVerticalDiffusivity(), | ||
| timestepper = :SplitRungeKutta3, |
| # time-stepping with a Courant-Freidrichs-Lewy (CFL) number of 0.7. | ||
| # Note that the suitability of the wizard depends on the model's `timestepper`. | ||
| # In this case we use a time discretization (Runge-Kutta) | ||
| # which allows the use of a variable time-step |
There was a problem hiding this comment.
the discussion is good, but I am not sure most users will understand. It could be better to simply make RK3 the default, and illustrate adaptive time-stepping. Users won't have to think about it --- they will just know that with the default setup, adaptive time-stepping is good to go 👍
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ncc/default-SplitRungeKutta3 #5248 +/- ##
=============================================================
Coverage 68.37% 68.37%
=============================================================
Files 394 394
Lines 21352 21352
=============================================================
Hits 14599 14599
Misses 6753 6753
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
merge? |
|
or how about merge into 5283 |
3 participants
It was using the time-step wizard with an Adams-Bashforth which is not recommended (because it changes timestepper under the hood when the wizard is called).
This PR fixes the example