[overfelt]

Checklist

• Documentation:
  • Design document has been generated and added to the docs
  • User's Guide has been updated
  • Developer's Guide has been updated
  • Documentation has been built locally and changes look as expected
• Linting
  • Pre-commit run locally
• Building
  • CMake build does not produce any new warnings from changes in this PR
• Testing
  • Add a comment to the PR titled Testing with the following:
    • Which machines CTest unit tests
      have been run on and indicate that are all passing.
    • The Polaris omega_pr test suite
      has passed, using the Polaris e3sm_submodules/Omega baseline
    • Document machine(s), compiler(s), and the build path(s) used for -p for both the baseline (Polaris e3sm_submodules/Omega) and the PR build
    • Indicate "All tests passed" or document failing tests
    • Document testing used to verify the changes including any tests that are added/modified/impacted.
    • Performance related PRs: Please include a relevant PACE experiment link documenting performance before and after.
  • New tests:
    • CTest unit tests for new features have been added per the approved design.
    • Polaris tests for new features have been added per the approved design (and included in a test suite)
• Stealth Features
  • If any stealth features are included in the PR, please confirm that they have been documented.

[cbegeman]

@overfelt I think in the meeting you mentioned success comparing the sphere_transport tests with MPAS-O? When this is ready for review, it would be great to see the convergence plots. Let me know if there are any hang-ups with the tests, as I imagine minimally namelist updates will be needed with this PR. I'm also thinking we would add versions of the tests that use FCT in addition to the current tests that do not, which would be a follow-on polaris PR with a submodule update to this PR after it is merged into Omega.

[overfelt]

Testing:
CTest unit tests run on chrysalis.lcrc.anl.gov with no new failures. The three new tests: TEND_PLANE_TEST, TEND_SPHERE_TEST and TEND_PLANE_SINGLE_PRECISION_TEST are calibrated to pass on Chrysalis.

Polaris omega_pr run against baselines on Chrysalis and passed.

The TEND_PLANE_TEST and TEND_SPHERE_TEST test all of the new functions added for higher order FCT.

There is no previous higher order FCT to compare against.

[xylar]

Some notes from Claude Opus from testing horizontal FCT on a global configuration

Feedback for the overfelt/FCTHigherOrderTendency PR, from trying the branch
at commit 4c23f7cb6a on a global EC30to60E2r2 case (Polaris
ocean/spherical/realistic_global/.../analysis_members_test, 5 days,
dt = 45 s, 384 MPI tasks on Chrysalis).

Motivation for trying it: Omega's unlimited horizontal tracer advection drives
the global minimum temperature from -1.95 C to -7.0 C over six days on this
configuration and is still falling, while MPAS-Ocean's monotonic limiter holds
its extrema flat over the same run. So this PR is very welcome.

The configuration that worked:

Omega:
  Advection:
    HorzTracerFluxOrder: 3
    Coef3rdOrder: 0.25
    HorzTracerFluxLimiterEnable: true
    HorzTracerFluxLimiterBudgetsEnable: false
    HorzTracerFluxLimiterMonotonicityCheckEnable: false

Two things I hit that might be worth addressing before merge.

1. HorzTracerFluxOrder: 2 with the limiter enabled uses unallocated arrays

This is the one that would bite a user, because order 2 is what a lot of
existing configurations already have set, so "turn the limiter on" is a
one-line change that silently lands here.

Tendencies.cpp:218-221 sets ForceLowOrder when the order is 2:

if (Order == 2) {
   this->TracerHorzAdv.ForceLowOrder = true;
   this->TracerHorzAdv.Coef3rdOrder  = 0;
}

and then reads the limiter flag independently at Tendencies.cpp:230, with no
check that the two are compatible:

Err += AdvectConfig.get("HorzTracerFluxLimiterEnable", TracerHorzAdv.FCT);

But TracerHorzAdvOnCell::init() returns early on ForceLowOrder
(TendencyTerms.cpp:139-144):

if (ForceLowOrder) {
   // Return when the 2nd-order tracer horz adv
   deepCopy(NAdvCellsForEdge, 0);
   deepCopy(AdvMaskHighOrder, 0);
   return;
}

and the FCT allocation block (HProvInv, HNewInv, HProv, TracerCur,
TracerMax, TracerMin, HighOrderFlx, LowOrderFlx, WorkTend, FlxIn,
...) sits after that early return, at TendencyTerms.cpp:160. So with
HorzTracerFluxOrder: 2 and HorzTracerFluxLimiterEnable: true, FCT is
true, every FCT kernel runs, and all of the FCT arrays are default-constructed
zero-extent views.

I did not run this combination, so I can't say whether it segfaults or
silently produces garbage -- but neither is a good outcome, and there is no
test covering it: TendencyTermsTest.cpp:1209-1210 pairs FCT = true with
ForceLowOrder = false, and :1313-1314 and :1341-1342 pair
ForceLowOrder = true with FCT = false. The true/true corner is untested.

Either of these would fix it:

• Reject the combination in Tendencies::init with an OMEGA_REQUIRE
  alongside the existing order check -- something like "FCT requires
  HorzTracerFluxOrder >= 3". Cheapest, and gives the user a clear message.
• Or move the if (FCT) allocation block above the ForceLowOrder early
  return, if order-2 FCT is meant to be supported (blending a 2nd-order
  high-order flux against the upwind low-order flux is what MPAS-Ocean does
  at config_horiz_tracer_adv_order = 2 with config_flux_limiter = 'monotonic', so users coming from MPAS-Ocean may well expect it to work).

A note in doc/userGuide/TendencyTerms.md next to
HorzTracerFluxLimiterEnable would help either way -- the table currently
documents the orders and the limiter independently, so there is nothing
telling the reader they interact.

2. printf in FCTMonotonicityCheck has three specifiers and two arguments

TendencyTerms.h:729-736:

if (TracerCur(ICell, K) < TracerMin(ICell, K) - Eps) {
   printf("Horizontal minimum out of bounds on tracer: %i %lg %lg\n",
          TracerMin(ICell, K), TracerCur(ICell, K));
}
if (TracerCur(ICell, K) > TracerMax(ICell, K) + Eps) {
   printf("Horizontal maximum out of bounds on tracer: %i %lg %lg\n",
          TracerMax(ICell, K), TracerCur(ICell, K));
}

Three conversion specifiers, two arguments, and the first argument is a Real
being consumed by %i. That is undefined behaviour and the printed numbers
will not be the intended ones. I assume the intent was ICell (or ICell, K)
followed by the two values.

Two smaller points on the same function while you are in there:

• It prints once per offending cell per layer per time step. On this
  configuration, with the overshoots that exist today, that would be a very
  large amount of output -- I left the check disabled for that reason rather
  than because of the format string. A per-step count, or a first-N-then-stop
  guard, would make it usable on a global mesh.
• A printf inside a Kokkos kernel is fine for a debug tool but will serialise
  badly on GPU; worth a comment saying it is diagnostic-only if that is the
  intent.

[cbegeman]

@overfelt Are you able to post relevant plots generated by the sphere_transport_with_viz test suite for both MPAS-O and Omega? Let me know if you need any help

[cbegeman]

@overfelt Can you rebase when you have a chance? Thanks!

[overfelt]

@cbegeman , I rebased the branch.

[overfelt]

These are the convergence plots from the ocean/spherical/icos/rotation_2d Polaris test. There are three tracers advected with mpas-ocean

Same with omega-ocean

[xylar]

Closed by accident, I assume?

[overfelt]

oops

[cbegeman]

Build issues on aurora, gpu:

Successful build with aurora, oneapi-ifxgpu, E3SM-Project/polaris@5546b58 + Omega submodule at c83fe0d

Unsuccessful build with aurora, oneapi-ifxgpu, E3SM-Project/polaris@8aa9ccb, Omega this branch 5e38631

Build log can be found here: /lus/flare/projects/E3SM_Dec/cbegeman/polaris-output/seamount-omega-fct-gpu-20260803/

Claude attributes the fail to https://github.com/overfelt/Omega/blob/d31b35395899bf9ae21671a7fa4abbf725f4dc7e/components/omega/src/ocn/TendencyTerms.h#L729-L744:

CUDA and HIP tolerate device-side printf; the Intel SYCL (DPC++) front end does not allow variadic calls in kernels at all, so this only shows up on the oneapi-ifxgpu / OMEGA_ARCH=SYCL Aurora build. That's why the same source builds fine on other machines/backends.

[mwarusz]

@overfelt

I ran CTests on pm-cpu with gnu and the following tests failed with a seg fault

19:IOSTREAM_TEST
27:STATE_TEST
42:FILL_VALUE_TEST
47:ANALYSIS_OP_TEST
48:ANALYSIS_SYS_TEST

The seg faults happen because the vertical advection module is not initialized in these tests. I opened #503 to fix this issue and to add checks for module dependencies.

CUDA and HIP tolerate device-side printf; the Intel SYCL (DPC++) front end does not allow variadic calls in kernels at all, so this only shows up on the oneapi-ifxgpu / OMEGA_ARCH=SYCL Aurora build. That's why the same source builds fine on other machines/backends.

You probably know this, but the fix is to use Kokkos::printf.

[mwarusz]

@overfelt

In addition to what we discussed offline, I have some minor suggestions and comments.

[sbrus89]

@mwarusz - thanks for reviewing. Are you ready to approve, or do you have more suggestions?

[sbrus89]

@overfelt, it looks like this is getting close! It seems like the CI is picking up some linting issues.

[overfelt]

@sbrus89 It looks like the lint check has problems installing the environment. The "install dependencies" step fails when trying to install conda.

[sbrus89]

@overfelt, I'm seeing 3 CTest fails on Frontier (craygnu). Most of these are probably just tolerances that need to be recalibrated, but there are some inf values reported.

TEND_PLANE_TEST

[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHProvInv LInf FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHProvInv L2 FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHNewInv LInf FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHNewInv L2 FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHighAndLowOrderFlux_High LInf FAIL, expected 6.938893903907228e-18, got 5.204170427930421e-18
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHighAndLowOrderFlux_High L2 FAIL, expected 5.762546266224026e-18, got 3.645353341218854e-18
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 LInf FAIL, expected 1.2480435640527313e-15, got 1.1987786865243336e-15
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 L2 FAIL, expected 6.665476716753009e-16, got 5.663839253635848e-16

TEND_PLANE_SINGLE_PRECISION_TEST

[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHProv LInf FAIL, expected 0, got 1.5258789e-05
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHProv L2 FAIL, expected 0, got 1.5258789e-05
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHProvInv LInf FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHProvInv L2 FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHNewInv LInf FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHNewInv L2 FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHighAndLowOrderFlux_Low LInf FAIL, expected 1.0095554e-15, got 0
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHighAndLowOrderFlux_Low L2 FAIL, expected 1.0095554e-15, got 0
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHighAndLowOrderFlux_High LInf FAIL, expected 6.938894e-18, got 1.8626451e-09
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHighAndLowOrderFlux_High L2 FAIL, expected 5.7625463e-18, got 1.643791e-09
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 LInf FAIL, expected 1.2480435e-15, got 7.405691e-07
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 L2 FAIL, expected 6.665477e-16, got 3.0637568e-07

TEND_SPHERE_TEST

[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHProvInv LInf FAIL, expected 3.0541724683419424e-05, got inf 
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHProvInv L2 FAIL, expected 1.0779233406323438e-06, got inf 
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTHNewInv LInf FAIL, expected 3.0541724683419424e-05, got inf 
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTHNewInv L2 FAIL, expected 1.0779233406323438e-06, got inf 
[error] [OceanTestCommon.h:676] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 LInf FAIL, expected 1.632639958580968e-15, got 1.521323597768628e-15
[error] [OceanTestCommon.h:681] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 L2 FAIL, expected 7.582979124216783e-16, got 7.094408556615681e-16

[overfelt]

@sbrus89 , I'll see if I can still access Frontier and try to replicate these failures. Some of the failing tests look like tolerances that can just be adjusted for Frontier but others are significant diffs.

[sbrus89]

Sounds good @overfelt, I'm also seeing similar fails on Frontier with craygnu-mphicc in case it's helpful for adjusting tolerances:

TEND_PLANE_TEST

[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHProvInv LInf FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHProvInv L2 FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHNewInv LInf FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHNewInv L2 FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHighAndLowOrderFlux_High LInf FAIL, expected 6.938893903907228e-18, got 1.0408340855860843e-17
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHighAndLowOrderFlux_High L2 FAIL, expected 5.762546266224026e-18, got 1.2836244918860016e-17
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 LInf FAIL, expected 1.2480435640527313e-15, got 9.524542988823467e-16
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 L2 FAIL, expected 6.665476716753009e-16, got 5.336197203674528e-16

TEND_PLANE_SINGLE_PRECISION_TEST

[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHProv LInf FAIL, expected 0, got 1.5258789e-05
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHProv L2 FAIL, expected 0, got 1.5258789e-05
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHProvInv LInf FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHProvInv L2 FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHNewInv LInf FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHNewInv L2 FAIL, expected 0, got inf 
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHighAndLowOrderFlux_Low LInf FAIL, expected 1.0095554e-15, got 0
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHighAndLowOrderFlux_Low L2 FAIL, expected 1.0095554e-15, got 0
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHighAndLowOrderFlux_High LInf FAIL, expected 6.938894e-18, got 1.8626451e-09
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHighAndLowOrderFlux_High L2 FAIL, expected 5.7625463e-18, got 1.7900343e-09
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 LInf FAIL, expected 1.2480435e-15, got 5.642431e-07
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 L2 FAIL, expected 6.665477e-16, got 2.4672764e-07

TEND_SPHERE_TEST

[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHProvInv LInf FAIL, expected 3.0541724683419424e-05, got inf 
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHProvInv L2 FAIL, expected 1.0779233406323438e-06, got inf 
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTHNewInv LInf FAIL, expected 3.0541724683419424e-05, got inf 
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTHNewInv L2 FAIL, expected 1.0779233406323438e-06, got inf 
[error] [OceanTestCommon.h:677] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 LInf FAIL, expected 1.632639958580968e-15, got 1.5120472343676013e-15
[error] [OceanTestCommon.h:682] TendencyTermsTest: FCTAccumulateHighOrderFlux_0 L2 FAIL, expected 7.582979124216783e-16, got 6.215666716489436e-16

I'll run on pm-cpu/gpu and post any fails I see as well.

[mwarusz]

Needs a couple of changes to consistently use transport velocity for tracers.

[mwarusz]

All of my comments have been addressed. CTests pass on pm-cpu (gnu) and pm-gpu (gnugpu). Moreover I ran ocean/spherical/icos/rotation_2d/with_viz using 3rd order monotonic transport and ocean/spherical/icos/divergent_2d/with_viz using 4th order monotonic transport with Omega and MPAS-O. The results are indistinguishable visually and almost identical in terms of convergence rates. For example,

MPAS-O divergent_2d

Omega divergent_2d

Approving. Thanks for your work on this @overfelt !

[andrewdnolan]

Thought it would be worth mentioning here: While running some initial G-Cases using #507, we ran into some unrealistic global maxima in temperature and salinity. After rebasing onto this branch and re-running with FCT turned on, the unrealistic global maxima have gone away!

Here are the performance changes for Omega between the runs, where everything else was consistent between the simulations:

-	Seconds per simulated day	Simulated years per day
No FCT	6.468	36.60
With FCT	7.157	33.07

[sbrus89]

With FCT turned off, this PR passes omega_pr on Frontier (craygnu) and on PM (gnu and gnugpu). There are machine precision diffs on Frontier with craygnu-mphipcc that will be blessed via polaris submodule update.

### Polaris `omega_pr` suite
- Baseline workdir: `/ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu-mphipcc_mpich/`
- Baseline build: `/ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu-mphipcc_mpich/build`
- PR build: `/ccs/home/brus/run/polaris_horz_fct_omega_pr_omega_craygnu-mphipcc_mpich/build`
- PR workdir: `/ccs/home/brus/run/polaris_horz_fct_omega_pr_omega_craygnu-mphipcc_mpich`
- Machine: `frontier`
- Partition: `batch`
- Compiler: `craygnu-mphipcc`
- Build type: `Release`
- Log: not found
- Result:
  - Diffs (2 of 24):
    - `ocean/spherical/icos/cosine_bell/decomp`
    - `ocean/spherical/icos/cosine_bell/restart`

normalVelocity       Time index: 0
0:  l1: 2.04236627610044e-12  l2: 9.73973798441671e-14  linf: 1.06581410364015e-14
  ^[[91mFAIL^[[0m /ccs/home/brus/run/polaris_horz_fct_omega_pr_omega_craygnu-mphipcc_mpich/ocean/spherical/icos/cosine_bell/decomp/24proc/output.nc
       /ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu-mphipcc_mpich/ocean/spherical/icos/cosine_bell/decomp/24proc/output.nc
tracer1              Time index: 0
0:  l1: 5.08817433011798e-13  l2: 1.41414872220080e-14  linf: 1.31838984174237e-15
  ^[[91mFAIL^[[0m /ccs/home/brus/run/polaris_horz_fct_omega_pr_omega_craygnu-mphipcc_mpich/ocean/spherical/icos/cosine_bell/decomp/24proc/output.nc
       /ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu-mphipcc_mpich/ocean/spherical/icos/cosine_bell/decomp/24proc/output.nc
          baseline comp.:   ^[[91mFAIL^[[0m
          runtime:          ^[[94m0:00:08^[[0m

normalVelocity       Time index: 0
0:  l1: 2.04236627610044e-12  l2: 9.73973798441671e-14  linf: 1.06581410364015e-14
  ^[[91mFAIL^[[0m /ccs/home/brus/run/polaris_horz_fct_omega_pr_omega_craygnu-mphipcc_mpich/ocean/spherical/icos/cosine_bell/restart/restart_run/output.nc
       /ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu-mphipcc_mpich/ocean/spherical/icos/cosine_bell/restart/restart_run/output.nc
tracer1              Time index: 0
0:  l1: 1.88622126004667e-16  l2: 1.24987568085725e-16  linf: 1.11022302462516e-16
  ^[[91mFAIL^[[0m /ccs/home/brus/run/polaris_horz_fct_omega_pr_omega_craygnu-mphipcc_mpich/ocean/spherical/icos/cosine_bell/restart/restart_run/output.nc
       /ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu-mphipcc_mpich/ocean/spherical/icos/cosine_bell/restart/restart_run/output.nc
          baseline comp.:   ^[[91mFAIL^[[0m
          runtime:          ^[[94m0:00:01^[[0m

I also tested ocean/planar/overflow/linear/zstar/smoke_test_horiz_adv_order_3 with and without FCT:

No FCT at 6 hours:

FCT at 6 hours:

[sbrus89]

This last commit is needed to make FCT the default in polaris in E3SM-Project/polaris#790

[sbrus89]

Passes CTests on Frontier (craygnu and craygnu-mphipcc)

[sbrus89]

Thanks @overfelt for all the great work!

[sbrus89]

Note that we should expect to see fails on several nightly tests until E3SM-Project/polaris#790 is merged. FCT is now on by default and current 2nd order cases do not turn it off in polaris. Running omega with 2nd order advection + FCT triggers a critical error in Omega.