TR-BDF2 Timestepper - #714
Conversation
Update stable for vn3.1
Merge vn3.2 stable
thomasmelvin
left a comment
There was a problem hiding this comment.
Looks good, i've made a few minor comments, mostly about the descriptions of namelist variables and the timestepping scheme (thanks for doing these!)
A more general question is could we get our existing siqn scheme from careful selection of choices in the TR-BDF2 scheme, i.e. making so that the step for the tr scheme is dt and for the bf2 to is 0. I think you default to the centred siqn scheme but maybe is possible a future change could be to unify these two schemes?
| description=Off-centering parameter for the Semi-Implicit timestepping scheme | ||
| help=This controls the implicitness of the Semi-Implicit timestepping scheme. | ||
| =A value of 0.5 corresponds to a Crank-Nicolson scheme, while a value of 1.0 | ||
| =corresponds to a fully implicit scheme. Values of 0.5 can be appropriate |
There was a problem hiding this comment.
Is this correct? Most of the gungho orographic tests would run with alpha = 0.5 without any problems. I'm not sure we have tried physics runs with alpha 0.5. I would suggest getting rid of the last sentance
There was a problem hiding this comment.
Thanks Tom, I was obviously confused about this point! Shallow water systems can suffer from orographic resonance with the centred scheme, which builds noise that can lead to an instability -- and I had assumed the compressible Euler equations did too. Out of interest I have now tried running the lfric_atm test-suite with alpha=0.5, and every job did actually complete!
I have removed this sentence and also edited the documentation
| Like ENDGame, GungHo uses a default of :math:`n_o=2` outer iterations and | ||
| :math:`n_i=2` inner iterations, for a total of 4 linear solves per timestep. | ||
|
|
||
| Because the transport terms are only updated in the outer loop, using more |
There was a problem hiding this comment.
I'm not sure this is quite right, after all we also transport u in the outer loop but don't zero the momentum residual in the inner loop. I would say that the reason we zero the density and potential temperature residuals in the inner loop is that the solver updates the transport tendency for these terms (that are only updated in the inner loop) compared to the momentum equation where the solver updates the nonlinear forcing terms (principally the pressure gradient) that are updated in the inner loop
There was a problem hiding this comment.
Somehow this got garbled between edits, thanks for pointing it out. I've tried to improve the explanation
|
This will require a linked-JEDI PR. I've made changes at: https://github.com/MetOffice/lfric-jedi/pull/1332 though I've not been able to test them yet due to this PR's branch conflicting with main. I'll approve once this is up-to-date and I can properly test. |
|
Your CLA signature was found on the base branch, but you appear to have modified the CONTRIBUTORS.md file in this PR. Please do not edit the CONTRIBUTORS.md file. If you have already signed the CLA, revert changes to the file and your signature will be picked up. |
PR Summary
Sci/Tech Reviewer: Alex Brown (@atb1995)
Code Reviewer: Pierre Siddall (@Pierre-siddall)
This PR adds a new timestepping scheme called TR-BDF2. Unlike the Semi-Implicit scheme currently used in Gungho, TR-BDF2 consists of two stages: TR (Trapezoidal) and BDF2 (Second-Order Backwards-Difference).
However both stages involve a Quasi-Newton outer/inner iteration like the Semi-Implicit scheme.
Because it contains two stages, the intention is that TR-BDF2 is used with a doubled timestep length. Physics schemes are called only once per the doubled timestep, reducing their cost. It is also possible to reduce the total number of inner iterations compared with the Semi-Implicit scheme, reducing the cost of the linear solver.
TR-BDF2 avoids the implicit off-centering of the Semi-Implicit scheme, which also improves the overall accuracy.
While this scheme will not be immediately adopted, it may be included in GC7 which is why I am lodging the code now.
The changes involved in this PR are:
inner_iterationssetting toinner_iterations_si, and makingthis an array (so that it is possible to perform different numbers of inner
iteration on each outer iteration)
stepperenumerator through to the transport scheme, as this theTR and BDF steps require different timestep lengths
different matrices are needed for the TR and BDF2 steps.
for any algorithm that calls the dynamics W2 mass matrices, such as
rhs_algIs Blocked By #659
Key Diffs
Therefore the appropriate diff for this PR can be found in this draft PR in my own
fork: https://github.com/tommbendall/lfric_apps/pull/18/changes
Documentation
equivalents can be found in this PDF:
results_tr_bdf2.pdf
provide reviewers with a path to view the compiled documentation through a browser
Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - test_tr_bdf2/run5
Suite Information
Task Information
✅ succeeded tasks - 1714
Security Considerations
Performance Impact
AI Assistance and Attribution
AI has been used to help draft the documentation.
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review