Add `SfcCoupling` class as an interface layer to the coupler by andrewdnolan · Pull Request #458 · E3SM-Project/Omega

andrewdnolan · 2026-07-02T22:12:41Z

Adds SfcCoupling, which manages the variables exchanged to (o2x) and from (x2o) the coupler for a sub-domain of the global mesh.

Changes

New SfcCoupling class supporting both MCT and MOAB coupling layouts
CouplingLayout Enum allows for CTesting of MCT (column major) and MOAB (row major) layouts.
Import/export of raw coupler data via unmanaged Kokkos views
applyImportFields() to push imported fields into Forcing
updateExportFields() to accumulate running averages of export fields on-device

Known limitations

Exported velocities use a placeholder constant pending vector reconstruction
- A small non-zero value is used to prevent infinite fluxes in the coupler

Checklist

Avg - for averaged fields Inst - for instantaneous fields (ie. SSH) Acc - for accumulated fields (to be added later)

Host array for the OcnToCplFields class were made private to protect from different units on host and device array. Needed to balance performance with correct units.

cbegeman · 2026-07-06T15:27:43Z

@andrewdnolan Besides the ctest, is testing this as a part of e3sm also possible (with cpl history output)?

andrewdnolan · 2026-07-06T15:44:18Z

@cbegeman Not quite yet, there will be some work to get the coupled driver branch hook up to use this branch. I can try to get that working, but that testing would be pretty ad-hoc. I think it'd be best to get this in with just the CTest testing, with the understanding that this PR is just a first pass, and a follow on will be needed to fully "hook things up".

That being said, I tried to be very thorough in the CTest and have mock pointers that mimic the raw attribute vectors passed by the coupler. I have tests for both unpacking (import) and packing (export) with both MCT and MOAB layouts. The data varies per-field and per cell, so a pretty reasonable test. I feel fairly confident in the code mechanics here, given the CTest, though there will absolutely be more work to get physically meaningful coupling. (But that will be dependent on a coupled driver).

cbegeman · 2026-07-06T15:47:08Z

@andrewdnolan Thanks for clarifying! That all sounds good.

cbegeman · 2026-07-06T16:40:48Z

+
+   // Kokkok views created with a label are zero-initialized by default.
+   // We reset the fields here anyway to be explicit about the fact that the
+   // OcnToCpl fields need to being a coupling interval with all zeros.


A missing word here in the comment?

Whoops, it should be "begin" not "being". With that I think the sentence makes sense. Thanks for catching that!

katsmith133 · 2026-07-06T17:35:58Z

+   deepCopy(InstSshCellH, SSHCellOwned);
+}
+
+// OcnToCpl fields need to being a coupling interval with all values set to 0.


I'm confused on what this is supposed to say... is it "...need to bring..."?

Whoops, "begin" not "being". Fixed. Thanks for catching that!

katsmith133 · 2026-07-06T17:39:51Z

+}
+
+// OcnToCpl fields need to being a coupling interval with all values set to 0.
+void OcnToCplFields::resetFields() {


Is resetting everything to 0 the safest thing here? Maybe I am misunderstanding, but is the a chance that these zeros could not be overwritten and get sent to the coupler?

Yes, we need to be able to reset all field to zero. The field contained in OcnToCpl are averaged fields, so each ocean time step (within the coupling interval) we calculate the update average. Given the average function: if we did not have a way to reset values to zero at the end of a coupling interval, these values would continue accumulating over the whole simulation, which is no good.

So this should only be called at then end of coupling interval, after the SfcCoupling::exportToCoupler has been called.

Ok, that makes sense. Now I understand the need to reset. I guess is there a change nothing gets accumulated in those averages and we send 0 to the coupler? I think for ssh or velocity that would be fine, but not so much for temp or salt.

The checks we have in place ensure zero values will never be exported to the coupler.

A rough sketch of what the coupled version of ocnRun will look like is in the design doc. We check to make sure the ocean timestep is less than the coupling interval and that they are evenly divisible (see here). So we are guaranteed that at least one iteration of the while loop in the coupled version of ocnRun will run, so at least one call to SfcCoupling::updateExportFields will happen (i.e. non zero export fields).

And OcnToCplField.resetFields() is called at the end of SfcCoupling::exportToCoupler(), so there's no confusion about when it should be called.

The coupled version of ocnRun will be implemented in the coupled driver PR, in the coming week(s), but this should set us up nicely for it.

katsmith133 · 2026-07-06T17:44:11Z

+   return Array;
+}
+
+// Shaed index formula for packing/unpacking from rae coupler buffer


"Shaed" = "Shared"? and "rae"?

Thanks for catching that. "rae" was meant to be "raw".

katsmith133 · 2026-07-06T19:01:27Z

Ran CTests on PM-CPU and GPU. All tests passed on CPU, but SFCCOUPLING_TEST failed on GPU. I don't see any error message pertaining to this failure and I don't see any compile messages that have to do with the files touched in this PR.

Rename SurfaceCoupling -> SfcCoupling throughout; document split CplToOcnFields/OcnToCplFields containers, Welford running-average accumulation, attachData's unmanaged strided views, and the import/apply/update/export method flow to match the implementation in #458. Document the once-per-interval unit conversion added in #458: device averages stay in native units for correct Welford math, conversion (temperature only; salinity is a TODO) happens once per interval in copyToHost via a local Teos10Eos, and the device arrays are private on OcnToCplFields so external code can't read unconverted data.

Co-authored-by: Carolyn Begeman <cbegeman@lanl.gov>

cbegeman · 2026-07-06T21:34:24Z

@andrewdnolan Let me know if it would be helpful to run the ctests on a particular machine. I imagine I should perhaps wait until we have a fix for the pm-gpu fail.

andrewdnolan · 2026-07-06T21:37:52Z

@cbegeman I will work on debugging the pm-gpu fails and running the full omega_pr suite first thing tomorrow. I've gotten a little more caught up testing other PR's than I had originally planned.

But agreed, probably not worth testing on another machine till that's addressed.

andrewdnolan · 2026-07-07T01:21:52Z

Testing

CTest unit tests

Machine: pm-gpu
Compiler: gnugpu
Build type: Release
Result: All tests passed

Polaris `omega_pr` suite

Baseline workdir: /pscratch/sd/a/anolan/omega_PR-458/baseline
Baseline build: /pscratch/sd/a/anolan/omega_PR-458/build/pm-cpu/baseline
PR build: /pscratch/sd/a/anolan/omega_PR-458/build/pm-cpu/surface-coupling
PR workdir: /pscratch/sd/a/anolan/omega_PR-458/surface-coupling
Machine: pm-cpu
Compiler: gnu
Build type: Release
Log: not found
Result: All tests passed

Tests added/modified/impacted

CTest: SFCCOUPLING_TEST — mocks unpacking (import) and packing (export) from raw pointers, validates averaging functionality, and applying coupler fields to the Forcing class

alicebarthel · 2026-07-07T22:50:44Z

+KOKKOS_INLINE_FUNCTION Real updateAverage(const Real OldAvg,
+                                          const Real NewValue,
+                                          const I4 NAccumSteps) {
+   return OldAvg + (NewValue - OldAvg) / (NAccumSteps + 1);


I had to check:
(NAccumSteps + 1) makes sense because NAccumSteps is updated after updateAverage() is called (via updateFields()).

Yeah NAccumSteps starts a coupling interval with a value of zero and is incremented at the end of the ocean timestep, so the +1 is necessary. This is following the MPAS-Ocean approach (see here).

alicebarthel

Approving after visual inspection.

alicebarthel

Approving after visual inspection.

cbegeman · 2026-07-08T14:49:51Z

@andrewdnolan Is this now ready to test on another machine? Any recommendations as to which?

sbrus89 · 2026-07-08T14:54:37Z

I'll test on Frontier today.

Co-authored-by: alicebarthel <36741014+alicebarthel@users.noreply.github.com>

alicebarthel · 2026-07-09T14:51:27Z

+   parallelFor(
+       {(int)AvgSfcTemperature.extent(0)}, KOKKOS_LAMBDA(int Cell) {
+          const Real Ct       = LocAvgSfcTemp(Cell);
+          const Real Sa       = LocAvgSfcSalinity(Cell);


@andrewdnolan It doesn't have to be added now but a first order absolute salinity --> practical salinity can be done using the factor I put in GlobalConstants.h:

Suggested change

const Real Sa = LocAvgSfcSalinity(Cell);

const Real Sa = LocAvgSfcSalinity(Cell) / Psu2Gpkg; // AbsSal to Practical conversion

The port that @katsmith133 was working on adds the spatial variability of the conversion, but that's second order. I'd recommend punting the full port to later.
Thoughts?

I'm inclined to hold off until a follow up PR. This first one is primarily focused on code mechanics so that we can build out the coupled driver. Once the first pass at the coupled driver is finished, we can return to this and work improve the physical fidelity of the coupling.

That's totally fine by me. I just wanted to flag this since I was talking about it to you and Kat both.

jonbob

We verified that the sequence changes in the initial routine are valid, by making ocn_comp_mct in mpaso work in a similar way and passing ERS tests

andrewdnolan · 2026-07-09T23:16:57Z

Testing

CTest unit tests

Machine:pm-cpu / pm-gpu
Compiler: gnu / gnugpu
Build type: Release
Result: All tests passed (on both machines)

Re-ran CTests on both pm-cpu and pm-gpu just to be double safe after the miscellaneous commits from code review.

andrewdnolan mentioned this pull request Jul 2, 2026

TimeMgr::Clock has no way to detach an Alarm, risking dangling pointers #459

Open

andrewdnolan added 21 commits July 6, 2026 08:08

Build out basic implementation of SfcCoupling

e8085ac

Add attachData and importFromCoupler methods

1fb44ed

Propogate Surface -> Sfc everyhere

a3cc9d3

Add applyImportFields method and refactor tests

036c393

Rename Meridional to Merid

63d6365

Support cpl pointer array larger than used fields

8c3ced2

Add getter for private member NAccumSteps

2865a74

Add compute method to OcnToCpl fields names

8ecbd56

Avg - for averaged fields Inst - for instantaneous fields (ie. SSH) Acc - for accumulated fields (to be added later)

Add updateExportFields method that runs on device

9141128

Fix bug in OcnToCpl stride for MCT layout

8e4420d

Make all test use a cell varrying expected value

cee980d

Make flatIdx function work for both import/export

ef68693

Add copyToHost and resetField methods to OcnToCpl

df98ebb

Add exportToCoupler method

fd24a41

Used isApprox for comparison

9c86c65

Add first pass at documentation

b6068e0

Fix up comments

75f4903

Fix bug where NImportFields was passed twice

a60dd31

Add test for non-default name and erasing

50bb8ba

Temp. conversion once per cpl interval on device

39f8e49

Host array for the OcnToCplFields class were made private to protect from different units on host and device array. Needed to balance performance with correct units.

Rename upate method and fix spelling errors

4205708

sbrus89 requested review from alicebarthel, cbegeman, jonbob and katsmith133 July 6, 2026 15:21

cbegeman reviewed Jul 6, 2026

View reviewed changes

Comment thread components/omega/src/ocn/SfcCoupling.h Outdated

cbegeman reviewed Jul 6, 2026

View reviewed changes

Comment thread components/omega/test/ocn/SfcCouplingTest.cpp Outdated

cbegeman reviewed Jul 6, 2026

View reviewed changes

Comment thread components/omega/test/ocn/SfcCouplingTest.cpp Outdated

katsmith133 reviewed Jul 6, 2026

View reviewed changes

Fix typos found in code review

b82337a

Apply suggestions from code review

1e8058c

Co-authored-by: Carolyn Begeman <cbegeman@lanl.gov>

andrewdnolan force-pushed the omega/surface-coupling branch from 6e673da to 1e8058c Compare July 6, 2026 21:06

Fix "being" / "begin" typo

089f50d

Access MinLayerCell on host in manual for loops

ada2701

alicebarthel reviewed Jul 7, 2026

View reviewed changes

Comment thread components/omega/src/ocn/SfcCoupling.cpp Outdated

alicebarthel reviewed Jul 7, 2026

View reviewed changes

Comment thread components/omega/src/ocn/SfcCoupling.cpp Outdated

alicebarthel reviewed Jul 7, 2026

View reviewed changes

alicebarthel reviewed Jul 8, 2026

View reviewed changes

alicebarthel approved these changes Jul 8, 2026

View reviewed changes

andrewdnolan commented Jul 8, 2026

View reviewed changes

Comment thread components/omega/test/CMakeLists.txt Outdated

Apply suggestions from code review

d0ac984

Co-authored-by: alicebarthel <36741014+alicebarthel@users.noreply.github.com>

andrewdnolan force-pushed the omega/surface-coupling branch from 3dfd90f to d0ac984 Compare July 8, 2026 20:38

alicebarthel reviewed Jul 9, 2026

View reviewed changes

jonbob approved these changes Jul 9, 2026

View reviewed changes

Switch to 8 tasks (from 1) for SFCOUPLING CTEST

c48126f

	const Real Sa = LocAvgSfcSalinity(Cell);
	const Real Sa = LocAvgSfcSalinity(Cell) / Psu2Gpkg; // AbsSal to Practical conversion

Uh oh!

Conversation

andrewdnolan commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Known limitations

Uh oh!

cbegeman commented Jul 6, 2026

Uh oh!

andrewdnolan commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cbegeman commented Jul 6, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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!

Uh oh!

katsmith133 commented Jul 6, 2026

Uh oh!

cbegeman commented Jul 6, 2026

Uh oh!

andrewdnolan commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewdnolan commented Jul 7, 2026

Testing

CTest unit tests

Polaris omega_pr suite

Tests added/modified/impacted

Uh oh!

Uh oh!

Uh oh!

alicebarthel Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewdnolan Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alicebarthel left a comment

Choose a reason for hiding this comment

Uh oh!

alicebarthel left a comment

Choose a reason for hiding this comment

Uh oh!

cbegeman commented Jul 8, 2026

Uh oh!

sbrus89 commented Jul 8, 2026

Uh oh!

Uh oh!

alicebarthel Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

andrewdnolan commented Jul 2, 2026 •

edited

Loading

andrewdnolan commented Jul 6, 2026 •

edited

Loading

andrewdnolan commented Jul 6, 2026 •

edited

Loading

Polaris `omega_pr` suite

alicebarthel Jul 7, 2026 •

edited

Loading

andrewdnolan Jul 8, 2026 •

edited

Loading

alicebarthel Jul 9, 2026 •

edited

Loading