Repository navigation
Conversation
shr_reprosum_calc returns the exact, correctly rounded sum of its inputs,
independent of the MPI decomposition. When reprosum_diffmax >= 0 it also
forms a plain floating point sum and shr_reprosum_tolExceeded warns if the
two differ by more than the tolerance. Five CAM call sites (the FV pole
averages in d2a3dikj and p_d_adjust, par_xsum, gmeanxy in mean_module, and
gmean_mod) treated that warning as a failure of the reproducible sum and,
with reprosum_recompute = .true., replaced it with a plain sequential sum
("old method"), or in gmean_mod aborted the run.
The plain sums are the inaccurate operand in that comparison, and both they
and the check itself depend on the decomposition. CAM's ERC test runs its
restart leg on half the tasks, so the check could trip on one leg only and
the fallback then injected a decomposition-dependent value. This is the
cause of the ERC_D_Ln9.f10_f10_mt232.FHIST_C5.izumi_nag.cam-outfrq3s_subcol
COMPARE_base_rest failure (ESCOMP/CAM issue ESCOMP#1514): the testmod sets
reprosum_diffmax = 1.0e-14 and reprosum_recompute = .true., the South Pole
wind average in d2a3dikj tripped only on the 12-task leg, and the pole-row
U/V then differed by one ulp. Subcolumns and the NAG compiler play no role.
Keep the warning (same pattern as fv_prints), drop every use of
shr_reprosum_recompute in CAM together with the duplicated fallback code
and gmean_float_norepro, and fix the "Nouth Pole" labels.
Answer impact: bit-for-bit with default settings (reprosum_diffmax < 0
disables the check, so the removed code never ran). Only runs that set
reprosum_diffmax >= 0 and reprosum_recompute = .true. can change, and only
at steps where the fallback fired; such events are visible as "exceeds
tolerance" messages in the baseline logs.
Assisted-by: claude-fable:5.1
Assisted-by: claude-fable:5.1
jimmielin
added
bug-fix
Collaborator
|
Claude found a fix for one of our tests that appeared after the shr_reprosum_calc refactor. Would you mind taking a look at this implementation? |
Member
|
Sorry, I'm not familiar enough with this aspect of the shr_reprosum code to be able to review this: I haven't used this feature of checking against the tolerance. |
Collaborator
|
@billsacks no problem! I'll ask around. |
Collaborator
|
The test passes I propose we simply remove the |
jimmielin
marked this pull request as draft
October 6, 2026 20:27
Collaborator
Collaborator
Author
|
closed as fixed in beta10 externals. Thanks @peverwhee ! |
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
6 participants
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.
Fixes #1514
Courtesy of Claude Fable 5.1.
It fixes the subcol restart rest but there is a one-time answer change for the physics load-balancing test as it is replaced by the exact (reproducible) sum rather than the old plain sum which is order-dependent: