PEM/ERP fixes for conusx4v1pg2_r05_oECv3 - #8554
Conversation
The ocean atm-ocn flux mesh (mbofxid) was an independent second load of the ocean coupler mesh (mboxid). Two independent MOAB loads (SQIJ/REPARTITION) give each rank the same ocean cells but a different local (handle) order, so the many by-index transfers between mboxid and mbofxid (atm-ocn flux xao, ofrac, domain copy, albedo, budget diagnostics) scattered data whenever the PE layout changed. This broke PEM and ERP for data-ocean F-cases (e.g. conusx4v1pg2_r05_oECv3 F2010-SCREAMv1). Make mbofxid an alias of mboxid (one shared mesh) so every by-index access is correct by construction, for data and active ocean alike. The only field carried by both o2x and xao states, So_fswpen, is renamed on the xao side to So_fswpen_ao to avoid a tag collision on the shared mesh. The xao->atm second-hop map reuses mboxid's o2x->atm comm graph, so no separate app id, context offset, or ComputeCommGraph is needed for mbofxid. Validated: PEM_Ln22 and ERP_Ln22 conusx4v1pg2_r05_oECv3 F2010-SCREAMv1-noAero now COMPARE bit-for-bit (cprnc 413 fields, 0 diffs; previously 159 differing fields).
In seq_rest_mb_write, the branch for land on its own masked grid (distinct from atm and rof) wrote fractions_lx and l2racc_lx without a global nx/ny, so PIO sized the global dimension from mblxid's packed active-cell count instead of the full land grid. That count is smaller than the maximum land GLOBAL_ID and scrambled lfrac/lfrin on restart. Pass the land grid dims (lnd_nx/lnd_ny from infodata), matching the cpl.hi history write in seq_hist_mod.
Aliasing mbofxid to mboxid removed the second-copy domain-tag copy, which was the only user of tagname, tagValues, arrsize, nloc and the shr_moab_mod (mbGetnCells/mbGetCellTagVals/mbSetCellTagVals) import in this routine. Drop the now-unused declarations to silence compiler warnings.
|
PEM_Vmoab_Ld3.ne4pg2_oQU480.F1850 and |
The issue is two-part in nature. One problem was in the OCN model decomposition, and the other was in the LND restart (I think you may have missed this one before). |
There was a problem hiding this comment.
Pull request overview
This PR fixes MOAB-coupler decomposition-dependent scatter in atm–ocn flux handling by ensuring the “flux” ocean mesh (mbofxid) is no longer an independently loaded/partitioned copy, but instead aliases the primary ocean coupler mesh (mboxid). It also addresses a restart I/O sizing issue for land fractions when land is on its own masked grid (not samegrid with atm or rof), preventing restart scrambling that broke PE-change tests.
Changes:
- Alias
mbofxid = mboxidand remove the second ocean mesh load/registration path to eliminate by-index scatter across PE layouts. - Rename the xao-side copy of
So_fswpentoSo_fswpen_ao(while keeping o2x asSo_fswpen) to avoid tag collisions on the now-shared mesh, and update flux writing/indexing accordingly. - Size non-samegrid land fraction restart writes using
lnd_nx/lnd_nyfrominfodata(instead of relying onmblxidglobal element counts).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| driver-moab/shr/seq_flds_mod.F90 | Renames xao So_fswpen to So_fswpen_ao and adds metadata to avoid tag collisions on the aliased ocean mesh. |
| driver-moab/main/seq_rest_mod.F90 | Uses lnd_nx/lnd_ny for non-samegrid land fraction/l2racc restart writes to prevent restart scrambling. |
| driver-moab/main/seq_flux_mct.F90 | Updates xao indexing and MOAB tag writes to use So_fswpen_ao on the flux mesh. |
| driver-moab/main/prep_atm_mod.F90 | Reuses mboxid’s comm graph/contexts for the xao→atm “second hop” now that mbofxid aliases mboxid. |
| driver-moab/main/cplcomp_exchange_mod.F90 | Removes the “second ocean copy” creation and makes mbofxid an alias of mboxid on coupler PEs. |
|
The land restart was fixed in #8541. |
vijaysm
changed the title
vijaysm
added
Coupler
|
Oh I see. The conus case is weird because its "fully coupled" but the river model is a stub. That case was indeed still not handled in the restart. |
|
Can you write a shorter description (above the detailed one so keep that) ? |
|
Now I'm wondering if the land restart issue was the only reason this failed since its an oddball case with a stub river. |
Change MOAB-coupler ATM-OCN flux handling by removing the extra copy of the ocean mesh in the coupler and associated comms. This fixes the ERP/PEM fails seen with regionally refined atm meshes. It also addresses a restart I/O sizing issue when land is on its own masked grid and the river model is a stub. [BFB] Fixes #8543
Change MOAB-coupler ATM-OCN flux handling by removing the extra copy of the ocean mesh in the coupler
and associated comms. This fixes the ERP/PEM fails seen with regionally refined atm meshes. It also addresses a
restart I/O sizing issue when land is on its own masked grid and the river model is a stub.
[BFB] but changes field list.
Fixes #8543
Details
This pull request makes significant improvements to the handling of the ocean "second copy" mesh and the associated flux fields in the MOAB-based coupler. The main change is to treat the second ocean mesh (
mbofxid) as an alias of the primary ocean mesh (mboxid), eliminating the need for a separate mesh load and context offset. This resolves issues with inconsistent mesh ordering and tag collisions, especially for theSo_fswpenfield, which is now uniquely named on the flux mesh. We also expect a bump up in performance since redundant copies of the ocean mesh, its I/O or migration from component PEs, construction and use of separate communication graphs etc are all now eliminated.Additionally, the pull request improves the handling of land grid dimensions for restart I/O when land is on a separate grid. We now size the land-fraction restart write by the land grid dimensions when land is on its own masked grid, a case the same grid branches do not cover. PEM is now bit-for-bit and ERP passes for both conus and ne30pg2 data-ocean F-cases.
Ocean mesh and field aliasing improvements:
mbofxid) is now an alias of the primary mesh (mboxid), removing the need for a separate load, app id, or context offset. This ensures consistent mesh ordering and correct by-index transfers for all ocean/atm fluxes. [1] [2] [3] [4]So_fswpenis renamed toSo_fswpen_aoon the xao state to avoid tag collisions on the shared mesh. All relevant code is updated to use the new name, and metadata is set accordingly. [1] [2] [3]Code cleanup and removal of obsolete logic:
OCN_SECOND_COPY_ID_OFFSETconstant and all logic related to explicit second-copy context offsets, as they're no longer needed with the mesh aliasing approach. [1] [2]Land grid restart I/O improvements:
lnd_nx,lnd_ny) frominfodatafor writing fractions and accumulators, ensuring compatibility with history output and preventing scrambled restarts. [1] [2] [3] [4]ROF_GRID=null, sosamegrid_alandsamegrid_lrare both false), thefractions_lx/l2racc_lxrestart write used no globalnx. PIO then sized the global dimension frommblxid's packed active-cell count, which is smaller than the maximum landGLOBAL_ID, scramblinglfrac/lfrinon restart. This is theelsebranch that PR Fix driver-moab land restart for cell vs point-cloud #8541's samegrid fixes do not reach.These changes simplify the codebase, improve robustness for different PE layouts and grid configurations, and resolve prior issues with field collisions and restart compatibility.
Validation
PEM_Ln22.conusx4v1pg2_r05_oECv3.F2010-SCREAMv1-noAero:COMPARE_base_modpesbit-for-bit (cprnc 413 fields, 0 diffs; previously 159 differing fields).ERP_Ln22.conusx4v1pg2_r05_oECv3.F2010-SCREAMv1-noAero:COMPARE_base_restPASS.ERP_Ln22.ne30pg2_r05_oECv3.F2010-SCREAMv1-noAero:COMPARE_base_restPASS.