Address remora-erf-driver issue 13 - #3887
Open
jmsexton03 wants to merge 3 commits into
Open
Conversation
- 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
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Coupled ocean SST was delivered as a land surface model. Selecting
OceanSurfset 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:t_surfwas frozen for the whole run.fill_tsurf_with_sst_and_tskis the only path that appliestsk_lev, and it was gated off;get_lsm_tsurfexplicitly skipped land. MOST then computed land heat and moisture fluxes against a static surface — no diurnal cycle, no synoptic evolution, nowrflowinpupdate atlow_time_interval.OceanSurf::Initseeds its array witherf.most.surf_temp(default 300 K) and nothing else ever wrote it, yetget_lsm_tsurfcopied it over every water cell every step. No diagnostic fired: the[260,320] Kcheck lives inApplyOceanSurfaceState, which one-way never calls, and 300 K passes it anyway.ApplyOceanSurfaceStatetook 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_sstafterfill_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 keepswrflowinpSST, and only the covered water cells are overwritten.ApplyOceanSurfaceStatetakes 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-domainsetVal, and beforefill_qsurf_with_qsat, which derives sea-surface humidity fromt_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::OceanSurfandSource/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_tsurftheta_typeselection block never reachedif (use_tsk) { m_ignore_sst = true; }under OceanSurf, because the OceanSurf print arm short-circuited past it. Once the lower-boundary fill is ungated, awrflowinprun carrying TSK would have taken the SST-interpolation arm where uncoupled ERF takes the TSK arm.ApplyOceanSurfaceStategated onlsm_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.GetOceanToAtmosSurfaceLayoutnow reportsba2d[0]/dmap[0]directly instead of reaching through the LSM data pointer. These were already the sameBoxArrayandDistributionMapping—grids[0]flattened withsetRange(2,0)ondmap[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:
t_surfacross the whole domaint_surfholds ERF's own valueA 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_sstfor two-way runs — is inerf-model/remora-erf-driver. That change depends on this one: it callsApplyOceanSurfaceStatewith the new coverage argument, so this needs to merge first.