Port IRRAD GridComp to MAPL3 - #94
Merged
Merged
Conversation
MAPL always creates Edge-staggered (VLOC=E) fields with Fortran bounds 1:LM+1, never 0:LM (confirmed in FieldCreate.F90's make_bounds_from_args). Both IRRAD and the Radiation container assume the 0:LM layer-interface convention throughout (e.g. FLX_INT(:,:,0)/(:,:,LM), PLE(:,:,0:LM-1)/(:,:,1:LM)), so every Edge pointer was silently off-by-one - undetected until now because the physics hadn't actually executed under -fcheck=all. Fixed by remapping each Edge pointer once, right after it's fetched, via a bounds-remap routed through a distinct CONTIGUOUS-declared scratch pointer (gfortran-15 rejects a self-referencing remap even on a CONTIGUOUS pointer, and POINTER+TARGET together is illegal Fortran - both confirmed with isolated test compiles): - GEOS_IrradGridComp.F90: all internal/import Edge fields in Run, all export Edge fields in Update_Flx (associated()-guarded, since exports can be null), and the RATS-diagnostics 4D arrays at both fetch sites. - GEOS_RadiationGridComp.F90: PLE/FLW/FLWCLR/FLWNA/FLA in Run. - Also fixes MAPL_GridGetInterior, which doesn't exist in MAPL3 - replaced with MAPL_GridGet(..., interior=...). Also: build GEOSradiation_GridComp (and GEOSirrad_GridComp) as SHARED libraries, matching the rest of MAPL3. See mapl3-porting-notes.md (this dir and GEOSirrad_GridComp/) for the full investigation and verification detail. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ad code - Add a real Initialize entry point that creates a clock-attached "irrad_lw_alarm" (ring interval from IRRAD_DT). Run now looks the alarm up by name each call and gates the full LW_Driver refresh on ESMF_AlarmIsRinging/ESMF_AlarmRingerOff, replacing the previous workaround of calling LW_Driver unconditionally every step (the MAPL2-style alarm fetch had been commented out with no MAPL3 equivalent noted at the time). - Collapse the NUM_BANDS/TOTAL_RAD_BANDS mismatch diagnostic into a single _FAIL(band_msg) call carrying one combined message, removing the three pflogger logger%info() calls it replaces. Drops the now- unused `use pflogger, only: logger_t => logger`, the `logger` local, and the `logger=` argument to MAPL_GridCompGet. - SetServices: replace the `if (n .ne. 0) then ... endif` wrapper around the RATS-diagnostics dynamic spec block with an early `_RETURN_UNLESS(n > 0)`, and register the Initialize/Run entry points unconditionally near the top of the routine so they no longer sit inside that conditional block. - Remove a broad set of unused locals and dead scaffolding from Run and its contained procedures: leftover MKL/mt19937 RNG types and their #ifdef HAVE_MKL branch, obsolete AERO/RRTMGP block-processing variables, RATNAMES, and a redundant "TOTAL" timer stop with no matching start. - Irrad_StateSpecs.rc: widen the ALIAS column for readability (no functional change). - Update mapl3-porting-notes.md (this dir and GEOSirrad_GridComp/) with this session's investigation and decisions.
- AERO import: 'AERO' is a nested State (aerosol-optics-provider bundle), not a plain Field, so it can't be expressed via Irrad_StateSpecs.rc (ITEMTYPE column only supports 'F'/'V'). Add it manually and unconditionally in SetServices via MAPL_GridCompAddSpec(itemtype=mapl_STATEITEM_STATE), and drop the row from the .rc file with a note explaining why. Previously the Field-typed spec left AERO unconnected, so ESMF_StateGet(import, 'AERO', ...) in Run/LW_Driver failed at runtime. - implements_aerosol_optics: temporarily hardcode to .false. instead of reading it via ESMF_InfoGet on the AERO state's Info, since the aerosol provider isn't populating that key in this configuration yet and the read was failing. Marked TODO pending a real fix. - TAUDIAG (LW_Driver): guard every read of TAUDIAG (used for TAUIR/ CLDTMP/CLDPRS diagnostics) behind USE_CHOU. TAUDIAG is a local, uninitialized array only filled by the IRRAD() call in the Chou branch; under RRTMG/RRTMGP (the current hardcoded default) it was read anyway, causing an FPE (floating invalid) on garbage stack values whenever any of those exports were requested. - irrad_lw_alarm interval: irrad_dt was read via MAPL_GridCompGetResource(gc, "IRRAD_DT", irrad_dt, _RC) with no default, so on configs without an explicit IRRAD_DT key it stayed uninitialized. That garbage became the alarm's ringInterval, which made ESMF_ClockAdvance hang on IRRAD's clock during the run loop (bisected via temporary debug prints in Cap.F90 and run_clock_advance.F90, since removed). Fix: get the real heartbeat straight from the clock via MAPL_ClockGet(clock, dt=run_dt, _RC), and use that (rounded) as the default for the IRRAD_DT override. - RRTMGP_cmake/CMakeLists.txt: link MKL_LIBRARIES into the RRTMGP target. The mklvsl RNG sources (mo_rng_mklvsl.F90/ mo_rng_mklvsl_plus.F90, built whenever MKL_FOUND) call vslNewStreamEx, but only NetCDF::NetCDF_Fortran was listed as a DEPENDENCIES, so the shared lib built fine but failed at runtime with "undefined symbol: vslnewstreamex_" the first time that code path ran.
Apply consistent Fortran style: normalize `end if`/`end do` keyword forms, spacing around operators and commas, relational operators (.eq./.ne./.gt. -> ==//=/>), array constructors ((/.../) -> [...]), and lowercase local variable/loop-index naming. No functional changes.
CTest-based standalone regression test for IRRAD, driven by a synthetic DataIRRAD import component, following the same esma_add_regression_tests pattern used by GWD and GOCART2G. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wrap remaining overlong lines to the file's 5-space continuation convention, fix TEST_ macro call indentation, lowercase MIN/MAX intrinsics, update stale AGCM.rc comments to irrad.yaml, and move the CO2_3d vmr-override call next to its sibling gas_concs%set_vmr calls for readability. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Read implements_aerosol_optics_method from the provider's Info instead of hardcoding .false. Add a minimal FakeGOCART test double (itemtype=STATE AERO export, implements_aerosol_optics_method=false) and wire it into irrad-sa so IRRAD's AERO import is actually satisfied.
The RATS_DIAGNOSTICS toggle list was parsed lazily on Run()'s first call via a save'd flag, a workaround from when this component had no Initialize() method. Now that it does, parse it there once and stash nRATS/nameRATS in the existing RRTMGP private state for Run() to read on every call.
Radiation_StateSpecs.rc: drop the ALIAS and COND columns, empty in every row. Irrad_StateSpecs.rc: re-align columns to their actual content width and drop the redundant duplicate "VARIABLE | DIMENSION | Additional Metadata" comment line above each table header. No schema/data changes.
parse_child in MAPL's ComponentSpecParser previously matched a truncated 'SetService' key due to a string-length bug (fixed in GEOS-ESM/MAPL#5307). These regression yamls were written to match that truncated key and need the trailing 's' restored now that the parser correctly requires the full 'setservices' key.
Replace the inline RADIATIVELY_ACTIVE_AEROSOLS block in LW_Driver with a call to the new compute_provider_aerosol_optics helper, and consolidate all aerosol-optics setup (SORAD band offset, TAUA/SSAA/ASYA allocation, and the provider call) into a single AEROSOLS-timed block that now runs right after the RATS-toggle setup, ahead of the unrelated cloud/emissivity bookkeeping it used to be interleaved with.
nameRATS/nRATS/TMP_R and the CLDTTLW/CLDHILW/CLDMDLW/CLDLOLW export pointers are only read inside the USE_RRTMG branch; the EG/FS/TG/TV/EV/RV emissivity and vegetation hardwiring is only read by the USE_CHOU branch's IRRAD() call; and PRS_LOW_MID/PRS_MID_HIGH aren't read until the LCLDMH/LCLDLM calc. All were being computed unconditionally before the SCHEME dispatch regardless of which radiation scheme was actually selected, or earlier than needed. Move each next to where it's used. CO2_FIXED, CWC/REFF, and LCLDMH/LCLDLM stay in the shared setup since they're read by more than one of the CHOU/RRTMGP/RRTMG branches, so relocating them would mean duplicating the setup rather than moving it. This is in the same spirit as the earlier (reverted) work that extracted the RRTMGP and RRTMG branches into their own Run_RRTMGP_LW/Run_RRTMG_LW routines: that attempt aimed for full isolation via extraction into separate subroutines, which turned out to be a bigger and riskier refactor than wanted. This keeps everything inline in LW_Driver and just relocates each SCHEME-specific setup step next to its own branch, getting the same "setup close to where it's needed" locality without the extraction risk.
RH's computation moved next to its only dependency, PL, and the whole AEROSOLS block (PL/RH setup, SORAD band offset, TAUA/SSAA/ASYA allocation, and the compute_provider_aerosol_optics call) moved to right after the MISC timer starts, ahead of the FCLD copy and CO2_FIXED handling that used to precede it. Neither block depends on the other in either direction, so this is a pure reorder: the AEROSOLS block only needs PLE/Q/T/IM/JM/LM/NB_IRRAD/ USE_RRTMGP_SORAD/USE_RRTMG_SORAD/import, none of which the FCLD/CO2_FIXED setup touches, and nothing downstream reads FCLD or CO2_FIXED before the SCHEME dispatch either.
LW_Driver has two separate blocks both timed as "MISC": the pre-SCHEME setup block (aerosol optics prep, FCLD/CO2_FIXED handling, CWC/REFF, LCLDMH/LCLDLM) and a separate post-SCHEME block (export pointers, TSREFF/SFCEM0/DSFDTS0/LWS0, aerosol array deallocation). Since MAPL timers accumulate by name, these were being lumped into one bucket in profiling output despite running at different points and doing different things. Rename only the first (setup) one to "LW-Setup" to tell them apart; the post-processing block stays "MISC".
…egression - Add MAPL2_UNDEF (1.0e15) and use it in place of MAPL_UNDEF throughout GEOS_IrradGridComp.F90. MAPL3's MAPL_UNDEF constant differs from MAPL2's original undef sentinel, so every undef comparison/fill (REFF defaults, CLDTMP/CLDPRS, OLCC5/LCSC5, the RRTMG-branch no-aerosol fallback fills, OLRA/OLA, LWSA/LAS, FLNSNA/FLNSA, and the OLRB band pointer fallback) was silently wrong. - Call set_inhomogeneity() from Initialize(), borrowed from GEOSradiation_GridCompMod::Initialize(). Without it, cloud_condensate_inhomogeneity's module-level state was never set, which drove a cloud_props_gpt%tau divergence in the McICA sub-gridscale condensate scaling step (zcw_lookup). TODO: this belongs to IRRAD's parent GridComp in production; revisit once the irrad-sa harness has a proper composite parent to own it. - irrad-sa: add IRRAD_DT to config, switch cmpchk.py to abs() diffs with masked-fill handling, and keep expdir around after a run instead of deleting it, for easier debugging. Together with additional exports in the MAPL2 baseline's History.rc (external to this repo), this achieves zero-diff for the irrad-sa regression test against the MAPL2 baseline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Label error. Requires at least 1 of: 0 diff, 0 diff trivial, Non 0-diff, 0 diff structural, 0-diff trivial, Not 0-diff, 0-diff, automatic, 0-diff uncoupled. Found: |
pchakraborty
added
0 diff
Point the GEOSgcm fixture checkout at release/MAPL-v3 instead of develop, since this branch targets the MAPL3 port and needs to build against MAPL3-compatible GEOSgcm sources. Turn off mepodevelop and persist_workspace (not needed without the GCM jobs), and comment out the AMIP and coupled GCM run jobs for now - the MAPL3 port isn't expected to run a full GCM yet, so CI stays scoped to the build and component-level regression tests (e.g. irrad-sa) instead of burning CI minutes on GCM runs that aren't ready to pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pchakraborty
changed the title
Mirrors the .circleci fixture change: pin the GEOSgcm fixture checkout to release/MAPL-v3 and turn off run-mepo-develop for both the standard and spack builds, since this branch needs to build against MAPL3-compatible GEOSgcm sources rather than develop. Also renames spack_build to spack_build_gcm, adds patch-esmf, and skips the spack install step (weird install issues on spack builds for now). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Issues that need to be resolved - #95
Summary
irrad-saregression test (root/cap/data yaml config,FakeGOCART_GridComptest-double for the AERO provider,cmpchk.pycomparison script) that now achieves zero-diff against a MAPL2 baseline.MAPL2_UNDEFvs MAPL3'sMAPL_UNDEF) and a missingset_inhomogeneity()call that were driving the last of the MAPL2/MAPL3 numerical divergence in the McICA cloud-sampling path.compute_provider_aerosol_optics, scheme-local setup moved closer to use, timer renaming) and StateSpecs.rc/SetServices cleanup along the way.Test plan
irrad-saregression test passes with zero-diff against the MAPL2 baseline