Fix inactive layer guard in gradient Richardson number computation by mwarusz · Pull Request #518 · E3SM-Project/Omega

mwarusz · 2026-08-17T22:45:06Z

Fixes #500

While fixing the Richardson number functor was trivial, adapting the vertical mixing test to this change turned out to be more complicated. I made the following changes to this test:

Removed explicit setting of CellsOnCell and NEdgesOnCell. This test in now passing with the arrays read from the mesh file.
Added a halo exchange after changing MaxLayerCell in testGradRichNum.
Set BruntVaisalaFreqSq and GeomZMid to fill values in inactive layers in testGradRichNum.
Moved the code that reset MinLayerCell and MaxLayerCell to 0 and NVertLayers, respectively, from testOneTwoOneFilter to the very end of testGradRichNum . I added a call to VertCood::minMaxLayerEdge right after to make the edge ranges consistent.

Checklist

Copilot

Pull request overview

This PR fixes an inactive-layer guard in the gradient Richardson number
computation (Omega vertical mixing), addressing Issue #500 by ensuring edge
contributions are skipped when they would access inactive layers.

Changes:

Update GradRichardsonNum to guard using MaxLayerEdgeTop (rather than the
deeper-side bound), preventing accesses into inactive layers.
Refactor testGradRichNum setup to rely more on mesh-provided connectivity,
add halo exchange after MaxLayerCell edits, and set inactive-layer data to
fill values to catch erroneous access.
Adjust test sequencing by resetting min/max layer bounds at the end of
testGradRichNum and refreshing edge ranges.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
components/omega/test/ocn/VertMixTest.cpp	Updates vertical mixing tests to reflect corrected inactive-layer guarding and halo/range handling.
components/omega/src/ocn/VertMix.h	Fixes the Richardson-number functor guard to use `MaxLayerEdgeTop` for inactive-layer safety.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

sbrus89 · 2026-08-18T16:22:52Z

@katsmith133, would you mind reviewing this?

mwarusz · 2026-08-18T19:03:43Z

Testing

CTest unit tests

Machine: frontier
Compiler: craygnu and craygnu-mphipcc
Build type: Release
Result: All tests passed

Polaris `omega_pr` suite

Baseline workdir: /lustre/orion/cli115/scratch/mwaruszewski/omega-pr-testing/richardson-fix/baseline-craygnu-mphipcc/
Baseline build: /lustre/orion/cli115/scratch/mwaruszewski/omega-pr-testing/richardson-fix/baseline-craygnu-mphipcc/build
PR build: /lustre/orion/cli115/scratch/mwaruszewski/omega-pr-testing/richardson-fix/build-craygnu-mphipcc
PR workdir: /lustre/orion/cli115/scratch/mwaruszewski/omega-pr-testing/richardson-fix/pr-craygnu-mphipcc
Machine: frontier
Partition: batch
Compiler: craygnu-mphipcc
Build type: Release
Log: /lustre/orion/cli115/scratch/mwaruszewski/omega-pr-testing/richardson-fix/pr-craygnu-mphipcc/polaris_omega_pr.o5302783
Result:
- Failures (1 of 22):
  - ocean/column/ekman

katsmith133 · 2026-08-18T20:31:59Z

@katsmith133, would you mind reviewing this?

Can do! Just did a quick read through of the code and it makes sense to me. I want to test it on PM. I can finish that up in the morning tomorrow.

sbrus89 · 2026-08-19T14:00:01Z

I believe PM is down today, but that would be great if you could test there. Thanks!

katsmith133 · 2026-08-20T18:13:44Z

Testing on Perlmutter

CTest unit tests (CPU):

Machine: PM-CPU
Compiler: gnu
Build type: Release
Result: All tests passed

CTest unit tests (GPU):

Machine: PM-GPU
Compiler: gnugpu
Build type: Release
Result: Failed to compile

I get the following compile errors with gnugpu on PM:

/pscratch/sd/k/katsmith/polaris-pr518/e3sm_submodules/Omega/components/omega/src/ocn/VertMix.cpp(480): error: The enclosing parent function ("applyVelVertMixImplicit") for an extended __host__ __device__ lambda cannot have private or protected access within its class
                                 (int, const TeamMember &Team) {

/pscratch/sd/k/katsmith/polaris-pr518/e3sm_submodules/Omega/components/omega/src/ocn/VertMix.cpp(587): error: The enclosing parent function ("applyTracerVertMixImplicit") for an extended __host__ __device__ lambda cannot have private or protected access within its class
                                    (int, const TeamMember &Team) {

Polaris `omega_pr` suite

Baseline workdir: /pscratch/sd/k/katsmith/polaris_testing_pr518//baseline_omega_pr
Baseline build: /pscratch/sd/k/katsmith/polaris-main/omega_build
PR build: /pscratch/sd/k/katsmith/polaris-pr518/omega_build
PR workdir: /pscratch/sd/k/katsmith/polaris_testing_pr518/pr518_omega_pr
Machine: pm-cpu
Compiler: gnu
Build type: Release
Log: not found
Result:
- Failures (1 of 22):
  - ocean/column/ekman

ocean/column/ekman test failure is expected.

mwarusz · 2026-08-20T18:43:45Z

@katsmith133

I get the following compile errors with gnugpu on PM:

This is #519 which got fixed in #520. To fix the build error this PR needed a rebase, which I just did.

katsmith133 · 2026-08-20T18:56:24Z

@katsmith133

I get the following compile errors with gnugpu on PM:

This is #519 which got fixed in #520. To fix the build error this PR needed a rebase, which I just did.

Ok, great, I will update and test again. Thanks!

katsmith133 · 2026-08-21T16:23:26Z

@mwarusz and @sbrus89 Since this currently has this issue in it: E3SM-Project#8650 and the PR that will pull that in to Omega (#521) is having some other issues to work out, would you prefer I hold off on testing this on PM-GPU? Or should I cherry pick E3SM-Project#8650's fix and test that way?

Note that after today I will be gone for a week on PTO, so someone else might need to do the PM-GPU testing if we wait for #521.

sbrus89 · 2026-08-21T16:31:45Z

@katsmith133, I think you can go ahead and approve based on your other testing. I can take care of testing on pm-gpu once #521 is merged. Thanks for your other testing on pm!

katsmith133

Approving based upon visual inspection and testing on PM, as well as testing by others on additional machines.

Assuming re-testing will pass on PM-GPU once #521 is merged.

sbrus89 · 2026-08-25T20:04:08Z

Local merge passes CTests and omega_pr suite on pm-cpu and pm-gpu

Polaris `omega_pr` suite

Baseline workdir: /global/homes/s/sbrus/scratch/polaris_baseline_omega_pr_omega_gnu_mpich/
Baseline build: /global/homes/s/sbrus/scratch/polaris_baseline_omega_pr_omega_gnu_mpich/build
PR build: /global/homes/s/sbrus/scratch/polaris_richardson_fix_omega_pr_omega_gnu_mpich/build
PR workdir: /global/homes/s/sbrus/scratch/polaris_richardson_fix_omega_pr_omega_gnu_mpich
Machine: pm-cpu
Compiler: gnu
Build type: Release
Log: not found
Result: All tests passed

Polaris `omega_pr` suite

Baseline workdir: /global/homes/s/sbrus/scratch/polaris_baseline_omega_pr_omega_gnugpu_mpich/
Baseline build: /global/homes/s/sbrus/scratch/polaris_baseline_omega_pr_omega_gnugpu_mpich/build
PR build: /global/homes/s/sbrus/scratch/polaris_richardson_fix_omega_pr_omega_gnugpu_mpich/build
PR workdir: /global/homes/s/sbrus/scratch/polaris_richardson_fix_omega_pr_omega_gnugpu_mpich
Machine: pm-gpu
Compiler: gnugpu
Build type: Release
Log: /global/homes/s/sbrus/scratch/polaris_richardson_fix_omega_pr_omega_gnugpu_mpich/polaris_omega_pr.o57602297
Result: All tests passed

mwarusz requested a lite review from Copilot August 17, 2026 22:49

Copilot started reviewing on behalf of mwarusz August 17, 2026 22:49 View session

Copilot AI reviewed Aug 17, 2026

View reviewed changes

Comment thread components/omega/test/ocn/VertMixTest.cpp

Comment thread components/omega/test/ocn/VertMixTest.cpp

mwarusz force-pushed the omega/richardson-fix branch from 3a56895 to 0f0b2e1 Compare August 17, 2026 23:09

sbrus89 requested a review from katsmith133 August 18, 2026 16:22

mwarusz added 2 commits August 20, 2026 12:40

Fix inactive layer guard in gradient Richardson number computation

777c7e5

Update vertical mixing test after Richardson number fix

edf086a

mwarusz force-pushed the omega/richardson-fix branch from 0f0b2e1 to edf086a Compare August 20, 2026 18:41

katsmith133 approved these changes Aug 21, 2026

View reviewed changes

sbrus89 merged commit e7dc83d into E3SM-Project:develop Aug 25, 2026
1 check passed

sbrus89 mentioned this pull request Aug 27, 2026

Update Omega submodule E3SM-Project/polaris#725

Merged

15 tasks

Uh oh!

Conversation

mwarusz commented Aug 17, 2026 • edited by sbrus89 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

sbrus89 commented Aug 18, 2026

Uh oh!

mwarusz commented Aug 18, 2026

Testing

CTest unit tests

Polaris omega_pr suite

Uh oh!

katsmith133 commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbrus89 commented Aug 19, 2026

Uh oh!

katsmith133 commented Aug 20, 2026

Testing on Perlmutter

CTest unit tests (CPU):

CTest unit tests (GPU):

Polaris omega_pr suite

Uh oh!

mwarusz commented Aug 20, 2026

Uh oh!

katsmith133 commented Aug 20, 2026

Uh oh!

katsmith133 commented Aug 21, 2026

Uh oh!

sbrus89 commented Aug 21, 2026

Uh oh!

katsmith133 left a comment

Choose a reason for hiding this comment

Uh oh!

sbrus89 commented Aug 25, 2026

Polaris omega_pr suite

Polaris omega_pr suite

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mwarusz commented Aug 17, 2026 •

edited by sbrus89

Loading

Polaris `omega_pr` suite

katsmith133 commented Aug 18, 2026 •

edited

Loading

Polaris `omega_pr` suite

Polaris `omega_pr` suite

Polaris `omega_pr` suite