plasim: broadcast nlowio and nstpw (deadlock and silent output corruption) - #53
Open
Sohex wants to merge 1 commit into
Open
plasim: broadcast nlowio and nstpw (deadlock and silent output corruption)#53Sohex wants to merge 1 commit into
Sohex wants to merge 1 commit into
Conversation
nlowio and nstpw are read from plasim_nl inside `if (mypid == NROOT)` in prolog, like every other namelist key, but unlike the keys around them they are never broadcast. plasimmod.f90 defaults nlowio to 1, so with NLOWIO = 0 in the namelist rank 0 holds 0 and every other rank holds 1, and the two halves of the model then disagree about which branch to take. That has two consequences on any run with more than one MPI task. FIRST, IT CAN DEADLOCK. outmod.f90's `if (nlowio .eq. 0)` blocks reach writegp/writesp, and writegp opens with the collective mpgagp. Where the branch is not symmetric, NROOT enters a collective no other rank enters. The run then sits in mismatched collectives at 100% CPU with no output past the file header, because OpenMPI busy-waits, so it looks like a slow run rather than a hung one. gdb on the ranks shows them split between MPI_Allreduce under fixer and MPI_Reduce under gridpointd. SECOND, AND WORSE, IT CAN SILENTLY CORRUPT OUTPUT. Where the branch IS symmetric -- and in outgp it is, an if/else whose two arms write the instantaneous and the accumulated version of the same fields -- the collective counts match, nothing hangs, and NROOT contributes its instantaneous slice to the same mpgagp gather that every other rank feeds an accumulated slice into. The assembled global field is part sample and part interval mean. That second one is measurable and was measured. Two binaries built from one source differing only in this fix, 320 steps at NLOWIO = 0 from the same restart on 16 ranks, T42: 474 of 8902 data records differ, every header identical. For surface temperature the only latitude rows that agree are 0-3, which is NLAT/NPRO = 64/16 = 4 and is exactly rank 0's slice, since mpgagp places rank r at offset r*NHOR. So 60 of 64 rows -- 93.75% of the grid -- carried interval means where instantaneous samples were intended, which is the opposite of what setting NLOWIO = 0 asks for. Worst affected were surface temperature by 14.93 K, cloud cover by 0.99, and albedo by 0.57. Fields written outside the if/else are byte-identical, which is the control on that result. The fix is to broadcast both keys in prolog beside the mpbci(nafter) already there. nafter is derived from them on NROOT and IS broadcast, which is why the output CADENCE was always consistent and only the branch was not -- and why this stayed invisible for as long as the namelist agreed with the compiled default. nstpw is used off-NROOT nowhere today, its one use being inside `if (mypid == NROOT)`, so broadcasting it changes nothing now and stops the same trap being reset later. At NLOWIO = 1 the change is a no-op by construction: the non-root tasks were already holding 1, which is what they now receive. Co-Authored-By: Claude Opus 5 (1M context) <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.
nlowioandnstpware read fromplasim_nlinsideif (mypid == NROOT)inprolog, like every neighbouring key, but unlike them they are never broadcast.plasimmod.f90defaultsnlowioto 1, so withNLOWIO = 0in the namelist rank 0 holds 0 and every other rank holds 1.On any run with more than one MPI task that has two consequences.
It can deadlock.
outmod.f90'sif (nlowio .eq. 0)blocks reachwritegp/writesp, andwritegpopens with the collectivempgagp. Where the branch is not symmetric, NROOT enters a collective no other rank enters. OpenMPI busy-waits, so it presents as a slow run rather than a hung one: 100% CPU, no output past the 40-byte header. gdb shows ranks split betweenMPI_AllreduceunderfixerandMPI_Reduceundergridpointd.Worse, it can silently corrupt output. Where the branch is symmetric -- and in
outgpit is, an if/else whose arms write the instantaneous and accumulated versions of the same fields -- collective counts match, nothing hangs, and NROOT feeds its instantaneous slice into the samempgagpgather that every other rank feeds an accumulated slice into.Measured with two binaries from one source differing only in this fix, 320 steps at
NLOWIO = 0from the same restart, 16 ranks, T42: 474 of 8902 data records differ, every header identical. For surface temperature the only latitude rows that agree are 0-3 --NLAT/NPRO = 64/16 = 4, exactly rank 0's slice, sincempgagpplaces rank r at offsetr*NHOR. So 60 of 64 rows, 93.75% of the grid, carried interval means where instantaneous samples were intended, which is the opposite of whatNLOWIO = 0asks for. Worst: surface temperature by 14.93 K, cloud cover by 0.99, albedo by 0.57. Fields written outside the if/else are byte-identical, which is the control.The fix broadcasts both keys beside the
mpbci(nafter)already there.nafteris derived from them on NROOT and is broadcast, which is why output cadence was always consistent and only the branch was not -- and why this stayed invisible while the namelist agreed with the compiled default.At
NLOWIO = 1it is a no-op by construction. Compiles cleanly.