Enable Omega time stepping with diagnostic computations#392
Merged
mark-petersen merged 6 commits intoE3SM-Project:developfrom May 4, 2026
Merged
Conversation
Author
TestingCTest unit tests
Polaris omega_pr regression suite
|
Author
|
Since this PR enables time stepping in Omega, more complex tests have been performed and compared with MPAS-O.
Baroclinic channel test (4 km): 
Namelist used: OmegaMPAS-O |
Author
|
Overflow test: Namelist used: OmegaMPAS-Ocean |
Author
|
Convergence test of time steppers
|
|
@hyungyukang, this is really fantastic! Could you make issues for each of the temporary fixes you added here so we don't lose track of them, and assign them to an appropriate person? |
Author
|
The results are identical across CPU core counts and across GPUs after merging #388. The CPU and GPU results differ slightly, as expected. Below are the domain-integrated KE values at day 10 of the overflow test on Perlmutter: |
Author
@xylar , Thanks! The changes I made were only meant for the tests above. The code I added in For MPAS-Ocean, the changes I made are not intended for general use. Other than the bug in the RK4 time stepper in MPAS-Ocean, they were used only for this test. I plan to open a PR for that bug in E3SM soon. I’ll provide all the details here soon. |
Author
|
Below is the workaround code I added separately at line 212 in |
4 tasks
Author
|
A long-term solution for the initial pseudo thickness from the Polaris will be provided through this Polaris PR: E3SM-Project/polaris#460 I tested the Polaris PR in combination with this time stepping PR using the overflow test, and the results are shown in this comment: E3SM-Project/polaris#460 (comment) |
mwarusz
reviewed
Apr 21, 2026
Member
mwarusz
left a comment
mwarusz
left a comment
There was a problem hiding this comment.
@hyungyukang Thanks ! This looks pretty good. Just a couple of comments.
Author
|
For reference, here are the changes I made on the MPAS-O side to match Omega as closely as possible:
|
@hyungyukang I think you might be able to achieve this with using |
@hyungyukang Can this be achieved with |
hyungyukang
force-pushed
the
omega/add-compute-diag-aux
branch
from
00c3696 to
c6edc1d
Compare
Author
@cbegeman , I tried that before, but both the overflow test and the QU240 spin-down test blew up with that option in MPAS-O. And as far as I know, it is also not available in Omega (correct me if I'm wrong). For now, the best option for making the two models as close as possible was to use |
Author
@cbegeman , yes, but in MPAS-Ocean it influences both horizontal and vertical advection, while in Omega it affects only vertical advection. Since horizontal FCT has not yet been implemented in Omega, I had to disable it in MPAS-O. If vertical FCT is turned off in both models, both the overflow test and the QU240 spin-down test blow up. |
What about the |
Thanks for clarifying. I didn't realize that you needed vertical FCT for these cases. |
|
Just making a note that my understanding is that we won't try to hook up p* in this PR. We'll get this in, I'll do the renaming PR (#327), and then we'll hook up p*. |
13 tasks
sbrus89
requested changes
Apr 29, 2026
Collaborator
|
Passes all cTests on Perlmutter with gnu on both CPU and GPUs. |
brian-oneill
approved these changes
Apr 30, 2026
brian-oneill
left a comment
brian-oneill
left a comment
There was a problem hiding this comment.
Approved. Remaining energy conservation concerns with 3rd and 4th order HorzAdv seem unrelated to this PR. Successfully passes ctests on Frontier CPU & GPU, as well as Improv.
mark-petersen
approved these changes
Apr 30, 2026
Collaborator
mark-petersen
left a comment
mark-petersen
left a comment
There was a problem hiding this comment.
Approving based on testing above - both comparisons with MPAS-O and convergence tests, which are the gold standard. Note that the overflow test above is the first instance where vertical advection is a leading-order term, and it was successful in comparison to MPAS-Ocean.
- Added computeMomDiagAux to AuxiliaryState.cpp to compute the diagnostic variables required for time stepping in Omega. - Removed some diagnostic variable computations before the PGF tendency to avoid duplicate work in computeDiagnosticAux. - Called computeMomDiag at the appropriate points during time stepping. - NOTE: SurfacePressure is currently handled temporarily in VertCoord.
- Reordered the computational procedures in Forward-Backward
This commit refactors momentum auxiliary state computation. - Renamed computeMomDiagAux to computeMomVertAux for clarity - Consolidated vertical auxiliary computations by calling computeMomVertAux within computeMomAux - Updated computeMomAux signature to accept TracerArray parameter
This commit restores the computational sequence in computeAllTendencies to its original order. - No changes to the computational sequence in computeAllTendencies are needed in this PR.
hyungyukang
force-pushed
the
omega/add-compute-diag-aux
branch
from
82ed710 to
3ecb506
Compare
sbrus89
approved these changes
Apr 30, 2026
Collaborator
There was a problem hiding this comment.
Passes all omega_pr tests on pm-cpu with a local merge into develop. Thanks for the great work @hyungyukang!
Polaris omega_pr suite
- Baseline workdir:
/global/homes/s/sbrus/scratch/polaris_v1_timestepping_omega_pr_baseline/ - Baseline build:
/global/homes/s/sbrus/scratch/polaris_v1_timestepping_omega_pr_baseline/build - PR build:
/global/homes/s/sbrus/scratch/polaris_v1_timestepping_omega_pr/build - PR workdir:
/global/homes/s/sbrus/scratch/polaris_v1_timestepping_omega_pr - Machine:
pm-cpu - Compiler:
gnu - Build type:
Release - Log: not found
- Result: All tests passed
mwarusz
approved these changes
Apr 30, 2026
|
@hyungyukang Just a reminder to update the checklist in the PR description when you have a chance |
Author
@cbegeman , Thanks for pointing that out. I just finished it. |
Author
@cbegeman , The fixed option seems to work, but the results are not as close as those from using Uniform together with the vertical velocity change in MPAS-O.
For now, it looks like the QU240 spin-down test can run without FCT if stronger del2 and del4 viscosity are used. However, the overflow test still seems to require at least vertical FCT. |
4 tasks
cbegeman
approved these changes
May 4, 2026
cbegeman
left a comment
cbegeman
left a comment
There was a problem hiding this comment.
Approving on the basis of successful testing of the omega_pr suite with a baseline of polaris's Omega submodule and a 1.5-day run on the overflow test case from E3SM-Project/polaris#572:
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
This PR enables time stepping in Omega by adding the diagnostic computations needed to evaluate the auxiliary diagnostic state required by the momentum equation.
computeMomDiagAuxtoAuxiliaryState.cppto compute the diagnostic variables needed for time stepping in Omega.computeMomDiagAux.computeMomDiagAuxat the appropriate points during time stepping.Forward-Backwardtime stepper.AuxiliaryStateTestwas slightly updated to avoid an error caused by calling Eos incomputeMomDiagAux. Although Eos is not directly tested inAuxiliaryStateTest, it still needs to be initialized for the test to run properly.SurfacePressureis currently handled temporarily inVertCoord.Checklist
Testingwith the following:have been run on and indicate that are all passing.
has passed, using the Polaris
e3sm_submodules/Omegabaseline-pfor both the baseline (Polarise3sm_submodules/Omega) and the PR build