Add centered pressure gradient implimentation by sbrus89 · Pull Request #8 · sbrus89/E3SM

sbrus89 · 2025-10-17T19:47:17Z

No description provided.

xylar

@sbrus89, a few comments. The main ones in terms of needing to discuss them relate to how the product of $\alpha p$ should be computed at layer interfaces and edges. I think it probably needs to be computed based on 4-point averages from cell centers and layer midpoints but we should make sure we're on the same page.

xylar · 2025-10-22T14:12:59Z

+   parallelFor(
+       "compute-interface-values", {NCellsAll, NVertLayers + 1},
+       KOKKOS_LAMBDA(I4 ICell, I4 KInterface) {
+          if (KInterface == 0) {


As we discussed, we need hierarchical parallelism here and this should be MinLayerCell or whatever the proper variable is, rather than 0.

Yes, I'll take care of swapping out for hierachical parallelism.

xylar · 2025-11-24T13:47:37Z

+             // AbsSalinityInterface(ICell, KInterface) =
+             //     AbsSalinity(ICell, KInterface);
+             SpecVolInterface(ICell, KInterface) = SpecVol(ICell, KInterface);
+          } else if (KInterface == NVertLayers + 1) {


Similarly, this should have hierarchical parallelism and use MaxLayerCell or similar.

xylar · 2025-11-24T14:04:35Z

+      const I4 ICell1      = CellsOnEdge(IEdge, 1);
+      const Real InvDcEdge = 1.0_Real / DcEdge(IEdge);
+
+      for (int KVec = 0; KVec < VecLength; ++KVec) {


This needs to be over something like MinLayerEdgeBot to MaxLayerEdgeTop with hierarchical parallelism.

xylar · 2025-11-24T14:06:33Z

+             (PressureInterface(ICell1, K) * SpecVolInterface(ICell1, K) +
+              PressureInterface(ICell0, K) * SpecVolInterface(ICell0, K));


Let's chat about it but I think we want the PAlpha product to be averaged both to cell centers and to layer interfaces.

Check out my latest commit and let me know what you think. The new averaging appeared to lower errors in my hydrostatic test.

…t#7866) This is changing code that is widely used and has been unchanged for a very long time, so I'm adding a lot of reviewers. Feel free to add more. I stumbled across this code when trying to debug a weird eamxx failure: I observed the following symtoms: If you look at components/elm/src/main/reweightMod.F90, you'll see the line SHR_ASSERT(bounds%level == BOUNDS_LEVEL_CLUMP, errMsg(__FILE__, __LINE__)) which causes this error: 1: free(): invalid pointer ... 0: #8 0x4c7e3ab in __shr_log_mod_MOD_shr_log_errmsg 0: at /pscratch/sd/a/acmetest/E3SM/share/util/shr_log_mod.F90:78 0: E3SM-Project#9 0x914149 in __reweightmod_MOD_reweight_wrapup 0: at /pscratch/sd/a/acmetest/E3SM/components/elm/src/main/reweightMod.F90:48 0: E3SM-Project#10 0x17bda7d in __dynsubgriddrivermod_MOD_dynsubgrid_wrapup_weight_changes 0: at /pscratch/sd/a/acmetest/E3SM/components/elm/src/dyn_subgrid/dynSubgridDriverMod.F90:403 0: E3SM-Project#11 0x17bf256 in __dynsubgriddrivermod_MOD_dynsubgrid_init._omp_fn.0 0: at /pscratch/sd/a/acmetest/E3SM/components/elm/src/dyn_subgrid/dynSubgridDriverMod.F90:150 I thought this was because the SHR_ASSERT was failing, but that's not the case (both bounds%level and BOUNDS_LEVEL_CLUMP are always 2). If I just comment this SHR_ASSERT out entirely, the test PASSES! The lowest line of code that triggers the free(): invalid pointer error is: shr_log_errMsg = 'ERROR in '//trim(file)//' at line '//toString(line) What I think is happening is that there is some memory corruption that is causing the freeing of the allocation done by toString to fail. So, this PR kind of sweeps that issue under the rug, but I do think it's better not to do dynamic allocations if you don't have to. The test PASSes with the changes in this PR. [BFB]

xylar · 2025-12-04T15:19:47Z

@sbrus89, your last few commits look reasonable to me. I would remove the commented-out loops after the switch to hierarchical parallelism eventually but it's okay to keep them for now while you make sure things are working as expected.

It's getting cumbersome to review here because your local omega/develop is out of date, so let's just got ahead and make the PR to Omega.

xylar · 2026-02-09T17:26:16Z

+
+            // compute specific volume from EOS
+            VCoord->computePressure(LayerThick, SurfacePressure);
+            DefEos->computeSpecVol(Temp, Salinity, PressureMid);


This is probably not where we want to be computing the specific volume in the long run, right? We would want to compute it somewhere external in the time step and just make it available here? I guess that's what Hyun is working on?

Right, I realized I need to add this call in Tendencies as well.

xylar · 2026-02-17T16:42:10Z

+      Real Rho0   = 1026.0_Real;
+      Real Gravity = 9.80616_Real;


This is not the right Gravity. We should use the value from GlobalConstants.h, which comes from here:
https://github.com/E3SM-Project/Omega/blob/develop/share/util_cxx/pcd_const.h#L28

xylar · 2026-02-17T16:42:24Z

+      const I4 ICell0      = CellsOnEdge(IEdge, 0);
+      const I4 ICell1      = CellsOnEdge(IEdge, 1);
+      const Real InvDcEdge = 1.0_Real / DcEdge(IEdge);
+      Real Rho0   = 1026.0_Real;


This isn't used and should be removed.

Surface flux terms, body forces and pressure terms were all getting multiplied by $\rho_0$, leading to incorrect dimensions and inconsistency with the rest of the derivation.

…1-equations Fixes to Omega v1 Governing Equations This pull request updates the documentation in OmegaV1GoverningEqns.md to improve the consistency, clarity, and correctness of the mathematical notation used in the derivation of the governing equations. The changes primarily remove unnecessary factors of \rho_0 from several equations, clarify the definition and notation for surface forces, and correct integration limits and variable usage for consistency. Mathematical and Notational Corrections: * Removed unnecessary factors of \rho_0 from the pseudo-height mass, tracer, and momentum conservation equations, as well as from their layer-averaged and Reynolds-averaged forms, to ensure dimensionally consistent and simplified expressions. * Clarified the definition of the surface force (traction vector) as \tau = \sigma \cdot {\bf n} and distinguished the horizontal component \tau_\perp in the momentum equation, improving the physical interpretation and accuracy of boundary force terms. Consistency and Integration Limit Corrections: * Corrected integration limits and variable usage in the layer and Reynolds-averaged tracer equations to ensure consistency with the rest of the document and the intended physical meaning. * Updated several equations to use consistent notation for averages, fluctuations, and layer integrals, improving readability and reducing the risk of confusion.

Use accessor functions to get state variables

Drop unused Rho0

sbrus89 force-pushed the omega/pgrad branch from 7062bf2 to 64da46a Compare October 17, 2025 19:51

xylar reviewed Nov 24, 2025

View reviewed changes

sbrus89 force-pushed the omega/pgrad branch from 58631a1 to 1377cf1 Compare December 2, 2025 21:33

sbrus89 force-pushed the omega/pgrad branch from 067c01a to 5947437 Compare January 8, 2026 16:48

xylar reviewed Feb 9, 2026

View reviewed changes

Comment thread components/omega/src/ocn/Tendencies.cpp

xylar reviewed Feb 9, 2026

View reviewed changes

mwarusz and others added 4 commits February 11, 2026 08:57

Use accessor functions to get state variables

3f1d8e3

Require valid time index in state getters

df74703

Require valid time index in tracer getters

82b0e7d

Update OceanState and Tracers documentation to reflect new getter API

ae37c58

xylar reviewed Feb 17, 2026

View reviewed changes

mwarusz and others added 8 commits February 20, 2026 11:32

Refactor Tracer::getByIndex and Tracers::getByName

95a4781

Update Tracers dev docs

015c2a4

Fix whitespace

91df52a

Fix incorrect $\rho_0$ multipliers up to (37)

b0d37f8

Surface flux terms, body forces and pressure terms were all getting multiplied by $\rho_0$, leading to incorrect dimensions and inconsistency with the rest of the derivation.

More fixes to the momentum derivation

5037c73

Clarify the horizontal gradient of Phi

32dfeaf

Fix more in Layer Equations and Discrete Equations

4d2feb2

sbrus89 force-pushed the omega/pgrad branch from 83d98d9 to 3418b2a Compare February 23, 2026 19:26

mwarusz and others added 7 commits February 25, 2026 09:41

Exchange tracer halos at every RK stage

dbe9dcf

Merge pull request E3SM-Project#341 from mwarusz/omega/fix-time-level

4f7b6a9

Use accessor functions to get state variables

Cleanup branch starting with new commit.

59b1666

Rename flag used to trigger higher order advection.

aec0440

Rename flag used to trigger higher order advection.

9f07fd9

Rename flag used to trigger higher order advection.

fc80b1b

Rename flag used to trigger higher order advection.

b73ed2e

sbrus89 and others added 20 commits March 16, 2026 12:56

Read PressureGradTendencyEnable and set PGrad enable flag

ee875b6

Lintng of Tendencies.cpp

91c6a74

Add temporary call to computeZHeight() in pgrad tendency

2f9bb86

Compute geometric z after specific volume

ad892fa

Lint pressure-gradient code

f541d5d

Use Gravity from GlobalConstants in PGrad

52d7f76

Drop unused Rho0

Initialize/clear Tendencies in IOStreamTest

4dbbc50

Add gradient of tidal potential and SAL

188ee35

Fix creation of tendency fields with same name

2fb9994

Initialize pressure gradient in state ctest

b9b3179

Fix IOStreamTest

bf37ee5

Fix PGradTest for GPU

ebf9999

Add pass/fail criteria to PGradTest

1421f7a

Lint files

e5acfee

Remove commented out code for debugging

2558656

Remove unsed member variables

70ae706

Fix for updated accessor state/tracer functions following rebase

5864ca3

Add user and dev documentation

e29cf6f

Fix compiler warnings from unsed variables

491b75c

Fix linting issues

9f02c7e

sbrus89 force-pushed the omega/pgrad branch from df991c7 to 9f02c7e Compare March 17, 2026 01:11

sbrus89 added 6 commits March 16, 2026 18:15

Fix linting issues after rebase

ee4be1a

Fix vertRange -> vertRangeChunked

e58f596

Fix converence criteria in CTest

45b90e5

Fix variable naming convention in PGradTest.cpp

f016d0f

Incorporate review feedback

bb0e1be

Fix linting issues

6f2b1a5

sbrus89 force-pushed the omega/pgrad branch from 5d2d748 to 6f2b1a5 Compare March 26, 2026 22:46

sbrus89 added 2 commits March 30, 2026 19:08

Use provis tracers for RK4

afd6126

Fix clang-format issues

bd19daa

		(PressureInterface(ICell1, K) * SpecVolInterface(ICell1, K) +
		PressureInterface(ICell0, K) * SpecVolInterface(ICell0, K));

Conversation

sbrus89 commented Oct 17, 2025

Uh oh!

xylar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xylar commented Dec 4, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants