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:27:57Z

+   std::map<std::string, int> ImportIdx;
+   std::map<std::string, int> ExportIdx;


Suggested change

std::map<std::string, int> ImportIdx;

std::map<std::string, int> ExportIdx;

std::map<std::string, int> ImportIdxMap;

std::map<std::string, int> ExportIdxMap;

I'd like to use some variable name here and elsewhere that implies this contains multiple field-index mappings. To me ImportIdx suggests a single integer index.

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?

cbegeman · 2026-07-06T16:57:13Z

+   int HaloErr = Halo::init();
+   if (HaloErr != 0) {
+      Err++;
+      LOG_ERROR("SfcCouplingTest: Error initializing defualt halo");


Suggested change

LOG_ERROR("SfcCouplingTest: Error initializing defualt halo");

LOG_ERROR("SfcCouplingTest: Error initializing default halo");

cbegeman · 2026-07-06T17:05:27Z

+   std::map<std::string, int> ImportIdx = {{"Foxx_taux", 3}, {"Foxx_tauy", 8}};
+   std::map<std::string, int> ExportIdx = {
+       {"So_t", 2}, {"So_s", 4}, {"So_u", 6}, {"So_v", 9}, {"So_ssh", 1}};
+};


What's the ultimate plan for where these maps would be set?

These maps will be created in the coupled driver; see here and here.

For the CTest's we just need maps where the key (string) matches the values provided from the coupler and then we just make up mock integer for testing. I've intentionally tried to make this non-continuous (i.e. uneven stride). I don't think we really need to have "mock" entries for every field, just a few per import and export, to test the packing and unpacking into the pointer arrays.

Got it. I agree. Thanks

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..."?

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.

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

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


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

katsmith133 · 2026-07-06T17:55:03Z

+   auto SfcStressZonal_ = CplToOcn.SfcStressZonal;
+   auto SfcStressMerid_ = CplToOcn.SfcStressMerid;
+
+   /// TODO: Shouldn't be making direct calls to Kokkos here.


Will this be a part of this PR or another follow on one?

This will have to be a follow on PR.

Updating the wrappers in src/infra/OmegaKokkos.h is a bit out of my comfort zone. Plus we'd need to support a hybrid (GPU + openmp) build, which is a larger can of worms.

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

+   ///< So_t    [K], in-situ approx (potential temp at P=0)
+   HostArray1DReal AvgSfcTemperatureH;
+
+   /// TODO: Export practical salinity (unitless) to coupler


Will this be a part of this PR or another follow on one?

EOS does not currently have a method for converting from absolute to practical salinity. So this will have to part of a follow on PR, after that's implemented.

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.

andrewdnolan added 19 commits June 25, 2026 14:26

Build out basic implementation of SfcCoupling

fc3c470

Add attachData and importFromCoupler methods

fcfcaa4

Propogate Surface -> Sfc everyhere

0c535de

Add applyImportFields method and refactor tests

3583ffb

Rename Meridional to Merid

27c1d0f

Support cpl pointer array larger than used fields

7cd18da

Add getter for private member NAccumSteps

5b1c7d5

Add compute method to OcnToCpl fields names

1ae8ff7

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

e84eee1

Fix bug in OcnToCpl stride for MCT layout

54a4d56

Make all test use a cell varrying expected value

aac4a83

Make flatIdx function work for both import/export

5670454

Add copyToHost and resetField methods to OcnToCpl

e1fd048

Add exportToCoupler method

7be65c5

Used isApprox for comparison

ececcec

Add first pass at documentation

be5d80f

Fix up comments

4a281cf

Fix bug where NImportFields was passed twice

a78e783

Add test for non-default name and erasing

6c1f301

andrewdnolan mentioned this pull request Jul 2, 2026

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

Open

andrewdnolan added 2 commits July 3, 2026 10:49

Temp. conversion once per cpl interval on device

5e7874f

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

1ac36a3

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

cbegeman reviewed Jul 6, 2026

View reviewed changes

katsmith133 reviewed Jul 6, 2026

View reviewed changes

		std::map<std::string, int> ImportIdx;
		std::map<std::string, int> ExportIdx;

	LOG_ERROR("SfcCouplingTest: Error initializing defualt halo");
	LOG_ERROR("SfcCouplingTest: Error initializing default halo");

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!

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!

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!

katsmith133 commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrewdnolan commented Jul 2, 2026 •

edited

Loading

andrewdnolan commented Jul 6, 2026 •

edited

Loading