Runge-Kutta-Legendre stabilised explicit method #1673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Uses Strang splitting (2nd order), with RK3-SSP for the advection parts, and Runge-Kutta-Legendre for the diffusion. This method can take timesteps considerably larger than a standard explicit scheme can for diffusive problems.
- Some changes to ensure that Arrays were always unique without having to reallocate. - Updated the IMEX/drift-wave and IMEX/advection-diffusion examples
|
Looks interesting! |
If a physics model is not split into convective and diffusive parts, should it be treated as convective or diffusive? Previous behaviour was to treat as diffusive. This switch keeps this default, but allows it to be changed.
Apparently I can't do mental arithmetic, so coefficients were off by a factor of 2 (and wrong sign in some cases...). Caught thanks to `test-solver`. Thanks @ZedThree!
Modify the MMS/time test so that it has non-zero convective and diffusive parts. This reduces the Karniadakis convergence order to 1, since it uses an Euler method for the diffusion part. Probably should remove this solver as there are better options. SplitRK converging at expected 2nd order.
Doesn't yet seem to work well, or at least is taking a lot more steps than should probably be necessary.
Large increases in timestep generally overshoot and result in reductions later. In some tests the timestep was oscillating between extremes. The default now limits changes in timestep to a factor of 2 (up or down).
Describes the options and includes links to paper describing the RKL method.
ZedThree
reviewed
May 7, 2019
ZedThree
reviewed
May 9, 2019
include/bout/solver.hxx
Outdated
| @@ -459,6 +459,9 @@ private: | |||
| /// Diffusive part (if split operator) | |||
| rhsfunc phys_diff{nullptr}; | |||
|
|
|||
| /// Should unsplit physics models be treated as diffusive? | |||
| bool unsplit_diffusive{true}; | |||
There was a problem hiding this comment.
How do you feel about is_unsplit_diffusive, or even is_nonsplit_model_diffusive?
There was a problem hiding this comment.
yes happy with either of those. Maybe is_nonsplit_model_diffusive is clearer?
ZedThree
previously approved these changes
May 9, 2019
Codacy complaining about variable `neq` not initialised in constructor. This initialises `neq` and other similar variables.
ZedThree
approved these changes
May 13, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uses Strang splitting (2nd order) to combine:
See https://doi.org/10.1016/j.jcp.2013.08.021
To try it out, set
solver:type=splitrk. Use optionssolver:timestepto set the internal timestep, andsolver:nstagesto set the number of stages in the RKL method.Explicit method, adaptive time stepping by default, checking accuracy a specified number of timesteps.
Tested by eye with a few examples, passes the unit and MMS tests. So far not particularly useful for real cases, but probably good enough to include.
Note that the
MMS/timetest has been modified to test both the convective and diffusive parts. This shows that Karniadakis converges at 1st order. I think we should remove that method, as there are better options (such as this).