+Add DETERMINE_TEMP_CONVERGENCE_BUG parameter #654
Merged
Conversation
This file contains 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
Added the new runtime parameter DETERMINE_TEMP_CONVERGENCE_BUG to avoid using layout-dependent tests for temperature and salinity tolerances to determine when to stop iterating in determine_temperature. Also added logic that correctly determines when iterations can be stopped because no further changes occur, and rearranged to loops to avoid revisiting layers that have converged. Because the default tolerances for the temperature, salinity and density changes are set to be the same and the partial derivatives of density with temperature and salinity are less than 1 in the MKS units used here for a realistic equation of state of seawater, this bug does not impact any of the existing regression test cases, but this bug could lead to non-reproducibility with non-default values. This commit changes the entries in the MOM_parameter_doc files that have Z_INIT_ALE_REMAPPING = .false. and FIT_TO_TARGET_DENSITY_IC = .true., by adding a new runtime parameter and by not logging DETERMINE_TEMP_T_TOLERANCE and DETERMINE_TEMP_T_TOLERANCE if they are not used because DETERMINE_TEMP_CONVERGENCE_BUG is false. By default, all answers are bitwise identical.
Hallberg-NOAA
added
bug
|
I agree wholeheartedly with these changes. This will effectively avoid layout dependent results after initialization for certain configurations and this bug should be disabled for new layered configurations with realistic initialization. |
marshallward
approved these changes
Jun 5, 2024
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/23731 ✔️ 🟡 |
This was referenced Nov 27, 2024
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.
Added the new runtime parameter
DETERMINE_TEMP_CONVERGENCE_BUGto avoid using layout-dependent tests for temperature and salinity tolerances to determine when to stop iterating indetermine_temperature(). Also added logic that correctly determines when iterations can be stopped because no further changes occur, and rearranged to loops to avoid revisiting layers that have converged.Because the default tolerances for the temperature, salinity and density changes are set to be the same and the partial derivatives of density with temperature and salinity are less than 1 in the MKS units used here for a realistic equation of state of seawater, this bug does not impact any of the existing regression test cases, but this bug could lead to non-reproducibility with non-default values.
This commit changes the entries in the MOM_parameter_doc files that have
Z_INIT_ALE_REMAPPING = .false.andFIT_TO_TARGET_DENSITY_IC = .true., by adding a new runtime parameter and by not loggingDETERMINE_TEMP_T_TOLERANCEandDETERMINE_TEMP_T_TOLERANCEif they are not used becauseDETERMINE_TEMP_CONVERGENCE_BUGis false. By default, all answers are bitwise identical.