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

Upgrade TimeStepWizard to support model spinup? #356

Closed
ali-ramadhan opened this issue Aug 13, 2019 · 4 comments
Closed

Upgrade TimeStepWizard to support model spinup? #356

ali-ramadhan opened this issue Aug 13, 2019 · 4 comments
Labels
feature 🌟 Something new and shiny
Milestone

Comments

@ali-ramadhan
Copy link
Member

@glwagner I'm thinking of upgrading the TimeStepWizard by adding a spinup_cfl and spinup_time to it, so it keeps CFL < spinup_cfl while model.clock.time < spinup_time. So it would also need a pointer to the model clock.

Does this make sense to you? Would be nice not to have to use two time-stepping loops.

@ali-ramadhan ali-ramadhan added the feature 🌟 Something new and shiny label Aug 13, 2019
@ali-ramadhan ali-ramadhan added this to the v1.0 milestone Aug 13, 2019
@glwagner
Copy link
Member

glwagner commented Aug 13, 2019

Consider:

  • The Adams-Bashforth time stepper is not well-suited for adaptive time-stepping, because it requires a initialization step. Thus it behooves scientists to update their time-step only infrequently.

  • However, during model spinup / transition to turbulence, a sharp change in flow regime may occur. This sharp change necessitates updating the time-step frequently to avoid blow up (an alternative strategy is to take uniform small time-steps during this time, which is safer but will take longer).

  • In addition to the need to update a time-step frequently near transition to turbulence, it is also wise to take conservatively small time-steps

Because of these considerations I am not sure it makes sense to add spinup_cfl, which only addresses the last, minor point. The main feature that is needed is a change in the frequency at which the time-step is updated (we currently do this manually by writing two loops). However, a change in update frequency is only really needed because Adams-Bashforth is ill-suited for adaptive time stepping (otherwise it'd be fine to update frequently even in the main loop). So it's a hack-on-a-hack. What do you think?

@ali-ramadhan
Copy link
Member Author

These are good points. I'll close the issue. I hope you don't mind if I use your comments in the documentation under the adaptive time-stepping section.

I ended up finding other ways to transition to turbulence in a stable manner using TimeStepWizard as is, e.g. either increasing the CFL with time

cfl(t) = min(0.005*t, 0.1)
wizard.cfl = cfl(model.clock.time)

or changing the number of time steps between adaptive time steps with time.

@glwagner
Copy link
Member

Yes, I’m happy to include general comments. We shouldn’t write too much on the topic. I think our advice will change when we have time stepping methods that are more appropriate for adaptive time stepping.

@glwagner
Copy link
Member

Ultimately I think it’d be nice to build adaptive time stepping into the time step function itself, once we have time stepping methods that are well suited for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🌟 Something new and shiny
Projects
None yet
Development

No branches or pull requests

2 participants