EAMxx: fix bugs related to ZonalMean diagnostic #7515
Merged
Conversation
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
The last bin must also contain the upper bound
bartgol
added
bug fix PR
BFB
cjvogl
approved these changes
Jul 14, 2025
There was a problem hiding this comment.
This looks good to me! I did indeed neglect to consider when a column would have a lat value exactly at the poles when deciding a bin would be "open" at the upper bound. The same issue is present in #7481, although it will manifest in a different way (namely that the column at the pole will be ignored)... so I'm glad you caught this now!
As far as the conflict with #7481, my vote is to merge this PR as-is to address the bug while #7481 is reviewed.
|
The failures in xyz_run_and_cmp are unrelated, and currently under investigation separately. I am going to merge this as I think the bug fix in the abstract grid method may impact other PRs. |
bartgol
added a commit
that referenced
this pull request
Jul 16, 2025
One bug was introduced by PR #7397, while two other were pre-dating that PR. [BFB]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
One bug was introduced by PR #7397 , while two other bugs was pre-dating that PR.
@cjvogl If you don't like the nano fix to your ZonalMean diag, another possible solution would be to make the bins span [-90.001,90.001], to ensure that ALL points are inside the bins (and avoid the call to
max). I think that for even values of ne, unit tests using np4 (gll) grid for physics will always get a column at the north/south poles. Of course, production grids are virtually always using pg2, for which is impossible to get a col exactly at the pole (it would be possible for pg3, but we don't use that).Fixes #7511