Skip to content

Address remora-erf-driver issue 13 - #3887

Open
jmsexton03 wants to merge 3 commits into
erf-model:developmentfrom
jmsexton03:fix/oceansurf-tsurf-authority
Open

Address remora-erf-driver issue 13#3887
jmsexton03 wants to merge 3 commits into
erf-model:developmentfrom
jmsexton03:fix/oceansurf-tsurf-authority

Conversation

@jmsexton03

Copy link
Copy Markdown
Collaborator

Coupled ocean SST was delivered as a land surface model. Selecting OceanSurf set one process-wide bool, m_has_ocean_lsm_tsurf, which simultaneously switched off the SST/TSK lower-boundary fill for every cell and gave the LSM authority over every water cell. Three defects followed from that one design choice:

  • Land t_surf was frozen for the whole run. fill_tsurf_with_sst_and_tsk is the only path that applies tsk_lev, and it was gated off; get_lsm_tsurf explicitly skipped land. MOST then computed land heat and moisture fluxes against a static surface — no diurnal cycle, no synoptic evolution, no wrflowinp update at low_time_interval.
  • One-way runs pinned the ocean to a constant. OceanSurf::Init seeds its array with erf.most.surf_temp (default 300 K) and nothing else ever wrote it, yet get_lsm_tsurf copied it over every water cell every step. No diagnostic fired: the [260,320] K check lives in ApplyOceanSurfaceState, which one-way never calls, and 300 K passes it anyway.
  • Partial coverage was unrepresentable. ApplyOceanSurfaceState took no mask and documented no sentinel, so where the ocean grid did not reach the atmosphere footprint the remap's zero fill arrived indistinguishable from a computed zero, and ERF had no way to keep its own SST there instead.

Coupled SST now has its own storage and is applied by SurfaceLayer::fill_tsurf_with_coupled_sst after fill_tsurf_with_sst_and_tsk, so the lower-boundary data is the base layer rather than an alternative to it: land keeps TSK, uncovered water keeps wrflowinp SST, and only the covered water cells are overwritten. ApplyOceanSurfaceState takes a per-cell coverage flag; absent, no cell is treated as covered, so one-way and uncoupled runs are inert by construction.

The apply is placed after update_surf_temp, which is a whole-domain setVal, and before fill_qsurf_with_qsat, which derives sea-surface humidity from t_surf.

Freeing the land surface model slot

Selection moves from the LSM enum to erf.use_coupled_sst. Coupled SST and a real land surface were previously the same slot, so it was one or the other — a coupled run can now also run Noah-MP. LandSurfaceType::OceanSurf and Source/LandSurfaceModel/OceanSurf/ are removed; a deck still carrying the old key aborts with a message naming the replacement rather than failing in the enum parse.

Fixes that fall out of deleting m_has_ocean_lsm_tsurf

  • The theta_type selection block never reached if (use_tsk) { m_ignore_sst = true; } under OceanSurf, because the OceanSurf print arm short-circuited past it. Once the lower-boundary fill is ungated, a wrflowinp run carrying TSK would have taken the SST-interpolation arm where uncoupled ERF takes the TSK arm.
  • ApplyOceanSurfaceState gated on lsm_type != OceanSurf, so a deck that misspelled the key had its ocean data dropped with no diagnostic. The gate is now the configuration flag, and declining to apply says so once.

GetOceanToAtmosSurfaceLayout now reports ba2d[0]/dmap[0] directly instead of reaching through the LSM data pointer. These were already the same BoxArray and DistributionMappinggrids[0] flattened with setRange(2,0) on dmap[0] — but nothing asserted it; now something does.

The coverage report replaces the whole-array min/max, which was dominated by the zero fill wherever the ocean did not reach and so could not distinguish a cold ocean from a missing one.

Verification

Run on the driver's coupled Upwelling deck:

case result
ocean and atmosphere share extents 1000 / 1000 surface cells covered
ocean shrunk to a quarter of the footprint 250 / 1000 covered — exactly geometric
t_surf across the whole domain stays physical; minimum never drops to the 0 K fill
one-way run no coupled SST applied at all; t_surf holds ERF's own value

A regression test asserting the covered/uncovered split, and confirmed to fail when coverage is claimed for every cell, is in the driver repo alongside the companion change.

Companion change

The driver-side half — deriving the per-cell coverage flag from the ocean→atmosphere remap stencils and injecting erf.use_coupled_sst for two-way runs — is in erf-model/remora-erf-driver. That change depends on this one: it calls ApplyOceanSurfaceState with the new coverage argument, so this needs to merge first.

- OceanSurf gated off the SST/TSK fill, freezing land t_surf all run
- One-way runs pinned every water cell to erf.most.surf_temp, silently
- Coupled SST now overwrites only covered water; wrflowinp is the base layer
- Selection moves to erf.use_coupled_sst, freeing the LSM slot for Noah-MP
- Coupled SST no longer routes through the LSM slot, so the class is dead
- Remove the enum value, the class, and its build and include entries
- Stale decks abort with a message pointing at erf.use_coupled_sst
- Document use_coupled_sst as independent of land_surface_model
@jmsexton03
jmsexton03 requested a review from hklion August 20, 2026 22:05
@jmsexton03

Copy link
Copy Markdown
Collaborator Author

This PR should be merged carefully relative to #3880 as the order that sea surface temperature gets written / overridden may be affected by how they are merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant