Chksum diag timestamp#879
Merged
marshallward merged 2 commits intoApr 16, 2025
Merged
Conversation
Hallberg-NOAA
approved these changes
Apr 16, 2025
Member
Hallberg-NOAA
left a comment
Hallberg-NOAA
left a comment
There was a problem hiding this comment.
These changes look correct to me, and I am expecting that this will be a valuable improvement in our error and change detection capability.
USE_DIAG_AS_CHKSUM will replace diagnostic calls with checksum data, enabling verification of diagnostic output. Although it is printed in the order of calculation, there is no information about the model state. If the post_data calls are reordered without changing the answers, then current testing will report an error, since it has no way of distinguishing between a reorder and an actual regression. This test appends a timestamp to the output. Time is based on the `Time_end` of the diag mediator. Format is "day seconds". This allows the output to be sorted for regression testing.
This patch uses the new timestamped chksum_diag output for sorting, so that diagnostics can be verified as order-independent. Sorting is achieved by copying the appending timestamp as a prepended timestamp, and then passed to sort. This is still a jury rigged solution, but it prevents false regressions from a reordered diagnostic.
Hallberg-NOAA
force-pushed
the
chksum_diag_timestamp
branch
from
2d26f1b to
c8782eb
Compare
Member
|
This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/27143. The two failing regression checks are expected due to changes in the format of the checksums that are being applied to evaluate whether diagnostics have changed. |
Member
Author
|
Manual overriding the regression tests. |
This was referenced Apr 23, 2025
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
2 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.
This PR has two commits:
Since it changes the
chksum_diagfiles, there will be a regression. But it should not propagate to future runs. It may, however, propagate to other forks, such as MOM-ocean.