Follow-ups from review of #4011: inert LSM wrfinput mapping, unreliable first-step radiation guard, test coverage

[asalmgren]

Follow-ups from the review of #4011 (multilevel refinement enhancements for the WRF-input pathway). The mechanical fixes from that review were pushed onto the PR branch; the three items below need a decision from the author rather than a patch, so they are collected here.

1. The LSM surface fields are read from a mapping that nothing populates

MakeNewLevelFromCoarse sizes lsm_data[lev] / lsm_data_name and calls lsm.Define() before the surface-only wrfinput read, with the comment that this is so the read can populate them (Source/ERF_MakeNewLevel.cpp:505-515). But lsm.Define() is an empty function for both land-surface models:

• Source/LandSurfaceModel/Noah-MP/ERF_NOAHMP.H:70
• Source/LandSurfaceModel/SLM/ERF_SLM.H:38

The structures the read actually indexes — LsmDataMap, LsmDataName, lsm_fab_data — are sized in NOAHMP::Init (Source/LandSurfaceModel/Noah-MP/ERF_NOAHMP_Init.cpp:55-110), which this path deliberately defers until after FillCoarsePatch. So at the time of the read, lsm.Get_DataIdx() iterates LsmDataName[idx] over an empty amrex::Vector and lsm_data[lev][lsm_idx] is still nullptr.

That does not fire today only because Lsm_WRFInputNames() returns an unpopulated wrfinput_map — nothing in Source/ ever fills it — which makes the LSM branch of the read dead code. The practical consequence is that the soil and vegetation fields the PR describes as being read from wrfinput_d02 (TSLB, SMOIS, SH2O, LAI, ZS, DZS, VEGFRA, TMN, SHDMIN, SHDMAX) are not in fact read. Note init_from_wrfinput already prints a warning about this when LSM is active.

Two ways out, and it is the author's call which is intended:

• If the LSM fields are meant to come from the file, populate wrfinput_map and move lsm.Init ahead of the read so the destination MultiFabs exist.
• If they are not, drop the pre-read Define block and the LSM entries from the surface-only variable list, so the code stops claiming to read something it does not.

2. istep[lev] == 0 is not a reliable "first step after this level was created" test

Source/TimeIntegration/ERF_AdvanceRadiation.cpp gates the "interpolate radiation from the parent instead of running RRTMGP" path on lev > 0 && istep[lev] == 0 && used_surface_only_init[lev].

istep is sized once in ReadParameters and is never reset in ClearLevel, so it retains the step count from the level's previous life. If level 1 exists for 50 steps, is de-refined away, and is later re-created by MakeNewLevelFromCoarse with erf.interp_atmos_from_coarse, then istep[1] == 50 and the guard silently does not apply — RRTMGP runs on exactly the thermodynamically inconsistent FillCoarsePatch state that ERF_MakeNewLevel.cpp:545-549 warns produces NaNs.

used_surface_only_init[lev] is also set at level creation and never cleared, so it stays 1 for the life of the run once a level has been built that way.

Suggested fix: set a flag in MakeNewLevelFromCoarse and clear it in advance_radiation once it has been consumed, rather than inferring "just created" from a step counter that outlives the level.

3. Did the reported tests exercise the new path?

At startup every level goes through MakeNewLevelFromScratch, where erf.interp_atmos_from_coarse is explicitly ignored with a warning (Source/ERF_MakeNewLevel.cpp:164-175). The PR describes Tests 1-3 as 2-level runs from wrfinput_d01 / wrfinput_d02 with erf.interp_atmos_from_coarse = true, which would take that ignore path rather than the new surface-only one.

It would be good to confirm how the surface-only path was reached in those runs (a regrid part-way through? amr.regrid_int?), and ideally to add a regression test that covers it, since none of the current tests do.

[adamwise95]

Resolution Summary

I've investigated all three items in this issue:

Item 1 (LSM surface field mapping):

• Confirmed that wrfinput_map has never been populated (framework hook from Read LSM data from WRFInput #2612)
• NoahMP reads its soil fields directly from wrfinput via the noahmpio Fortran library using nc_init_file
• The pre-read Define() block in MakeNewLevelFromCoarse was misleading but harmless (Define is empty)
• Fix: Clarified comments to explain that LSM reads its own state; no code behavior change needed

Item 2 (Radiation guard flag):

• Already fixed! The rad_interp_from_coarse_pending flag is properly set in MakeNewLevelFromCoarse and cleared in advance_radiation
• used_surface_only_init no longer exists

Item 3 (Test coverage):

• My initial description of my Test 1 was not comprehensive enough.
• Test 1 was manually test with both a same start and delayed start:
  • Delayed wrfinput_d02 start: correctly uses surface-only initialization
  • Same start time: correctly ignores interp_atmos_from_coarse with warning

Additionally fixed: Radiation interpolation crash when using interp_atmos_from_coarse - the removal of temporary MultiFabs in commit 0a1b353 caused assertion failures. Restored temporaries with ghost cells for safe interpolation.

PR incoming (will link once submitted)