Fix BT solver wide halo log_param and hifreq diagnostics timestamps#209
Merged
Conversation
* Description of wt_eta related variables * Longname of PFu_BT
Logging BT halo size parameters should happen after the modification in clone_MOM_domain, where they are lower-bounded by the domain halo. This commit fixes this and therefore may result a change of parameters in MOM_parameter_doc.layout in past runs.
This commit fixes the conflicting timestamps for diagnostics at the BT step level (do_hifreq_output=True). Instead of the actual dtbt, a nominal time interval is used to "squeeze" output at all nstep+nfilter steps into a time interval of a single baroclinic timestep.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #209 +/- ##
=========================================
Coverage 37.15% 37.15%
=========================================
Files 262 262
Lines 72744 72745 +1
Branches 13597 13597
=========================================
+ Hits 27031 27032 +1
Misses 40699 40699
Partials 5014 5014
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Hallberg-NOAA
approved these changes
Sep 20, 2022
Member
Hallberg-NOAA
left a comment
Hallberg-NOAA
left a comment
There was a problem hiding this comment.
The compression of the times for the high frequency output so that all of the substeps, including those in the extra filter time, strikes me as a very clever idea. The other changes in this PR are clearly correcting minor issues with this file.
Member
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/16833 ✔️ 🟡 |
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
3 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.
The first commit fixes a couple of typos in BT solver.
The second commit moves the
log_paramcalls of the wide halo size parameters after theclone_MOM_domaincall, where these parameters may be modified. Otherwise, the entries inMOM_parameter_doc.layoutrecord only the use specified values or the default, which is usually zero. As a result, this commit may changeMOM_parameter_doc.layoutfrom the old runs.The third commit addresses a potential problem with the timestamps of the BT step diagnostic outputs. Previously, the (hifreq) output between BT steps has a time interval =
dtbt. While this is technically the right choice, it ignores the filter steps. As a result, the time interval at each baroclinic time step is not enough for all output from nstep+nfilter BT step. The filter step output has a timestamp that exceeds the end time of the current baroclinic step. The output at the filter steps then is never saved (overwritten by the output from the next baroclinic time step with the same timestamps), except for the last baroclinic timestep of the run.This commit provides a walk-around. A nominal BT step size
dtbt_diagis introduced to "squeeze" all nstep+nfilter output into the same baroclinic step time interval.