Conversation
|
When running |
|
|
||
| for (int KVec = 0; KVec < KLen; ++KVec) { | ||
| const I4 K = KStart + KVec; | ||
| const Real WAvg = 0.5_Real * (LocTotVertVelocity(Cell1, K) + |
There was a problem hiding this comment.
Do we want to use
so that we can easily switch from anisotropic to isotropic interpolation for all variables (here and elsewhere)?
There was a problem hiding this comment.
We can’t use InterpCellToEdge here as is, since it takes an Array1DReal as input, while TotalVerticalVelocity is an Array2DReal. Supporting this would require either an overload or a templated version of the interpolator. It may make more sense to handle this outside this PR and identify any other places to implement this refactor.
|
See compiled documentation here: |
|
Using my time-stepping working branch (https://github.com/hyungyukang/Omega/tree/hkang/omega/merge-vert-vel As expected, I obtained a convergence rate of 1.971 with VerticalTracerFluxOrder: 2, which matches the convergence rate obtained in MPAS-Ocean:
|
| } else if (VAdv->VertAdvChoice == VertAdvOption::FCT) { | ||
| ThicknessForVAdv = AuxState->LayerThicknessAux.ProvThickness; | ||
| } | ||
| VAdv->computeTracerVAdvTend(TracerTend, TracerArray, ThicknessForVAdv, |
There was a problem hiding this comment.
| VAdv->computeTracerVAdvTend(TracerTend, TracerArray, ThicknessForVAdv, | |
| VAdv->computeTracerVAdvTend(LocTracerTend, TracerArray, ThicknessForVAdv, |
There was a problem hiding this comment.
I’m approving this PR based on my testing and visual inspection. All the changes I requested have been addressed, and the code works as expected.
@brian-oneill, thank you very much for your hard work on this!
mark-petersen
left a comment
There was a problem hiding this comment.
Approving based on Hyun's convergence test above, and visual inspection. Successfully passed all cTests on perlmutter CPU and GPU with gnu compilers.
|
Great work, @brian-oneill! I just wanted to check; has anyone yet verified the 3rd and 4th order convergence options? If not, I'll do this with the polaris merry-go-round test. |
@cbegeman , I ran it last time with the 3rd convergence option. However, I was not able to obtain the 3rd order convergence rate for both Omega (1.981) and MPAS-Ocean (1.981). Could you please run it as well just in case I missed something? |
|
Using one processor, I was able to reproduce @hyungyukang's results: Testing whether this is fixed by #316 |
@cbegeman , I ran the test on my end with 4th order advection without limiter and obtained the same convergence rate (1.980) for both models. |
|
There is currently a halo boundary error with the merry-go-round test, per a slack conversation with @cbegeman. Following rows are with 1, 4, and 8 CPUs. @brian-oneill confirmed that halos of normvel tendency and are initialized to correctly to 0. I suspect that the error at partition boundaries is due to the imposed horizontal velocity for this specific test. |
|
Update: Halo updates with RK2/RK4 fixed boundary error problems in previous post. |
|
4th order vertical advection with limiter (MPAS-O top, Omega bottom):
|

















Add the
VertAdvmodule to Omega, providing data structures and methods for vertical advection. This module includes:Additional changes in this PR:
parallelForOuterwith an optional argument to enable the use of scratch arrays in hierarchical parallelism.ProvThickness, which represents thickness after horizontal thickness flux. Further work required to fully implement provisional thickness.BottomDepthfromHorzMesh, this field was previously added toVertCoord.This PR builds and passes the unit tests successfully on Chrysalis, pm-cpu, pm-gpu, and Frontier (CPU & GPU).
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 buildFixes #338