NaN initialization fixes #1026
Merged
NaN initialization fixes #1026
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
The calculate_vertical_integrals function in MOM_diagnostics includes the calculation of density in the z_top and z_btm arrays. The values are initializes from is:ie and js:je, but the int_density_dz function expects these arrays to be defined from isq:ieq+1 and jsq:jeq+1. The reduced init range raises a FPE when signaling NaN initialization is turned on. Previously, these values would have presumably be set to zero. This patch extends the initialization of z_top and z_btm to prevent this issue.
Two function arguments in the wave_speed diagnostic were computed inside of the function call, which operated over the whole array and raised FPEs when initialized with NaNs. 1. The diagonal input igu+igl of the tridiagonal solver tdma6 is replaced with the precomputed Igd(:) = Igu(:) + Igl(:) 2. The Z_to_H rescaling of Hc(:) in remapping_core_h() is replaced with Hc_H, which precomptes the dimensional rescaling.
The net penetrating shortwave heat flux at the top level was using colon notation and a SUM() intrinsic, which was causing sums over NaN in Nan-initialized mode, as well as a potentially ambiguous summation ordering. This patch replaces the implicit update and summation with explicit sums over the reduced non-halo domain.
This patch explicitly sets the Drho_tol and x_tol tolerances to zero for the neutral diffusion unit tests. This was required to avoid FPEs during NaN-initialization, since the unit test does not formally initialize many variables.
A conditional test in advect_x of MOM_tracer_advect contained a logical
short-circuit:
(do_i(i)) .and. (Ihnew(i) > 0.0)
where Ihnew was unset if do_i was false. This caused an FPE in builds
where short-circuit logic checks were disabled and Ihnew was initialized
with NaN.
This patch fixes the issue by splitting the statement into two nested
if-statements.
This patch initializes the global adjusment and scaling factors of the surface forcings to zero. This prevents FPEs when initalizing with NaNs.
Two logical checks in MOM_entrain_diffusive were restructured to eliminate short-circuit logical checks which were raise when NaN initialization was enabled. In the first case, a new logical variable (do_entrain_eakb) was introduced to enable nested-and conditions. In the second case, a nested (k == kb) if-block inside of a (k >= kb) block was moved outside as a separate if-block.
Some of the logical expressions in set_viscous_BBL used to calculcate L(:) (cell width fraction) contained terms which depended on other terms being true and could not be evaluated without short-circuit logic This patch breaks some of the more complex expressions into smaller terms, and pre-computes the logical flags in order avoid logical evaluation FPEs which occur during NaN initialization.
A zonal density calculation which only updates from is-1:ie+1 was being used in an array update, which include uninitialized halo updates, which raised FPEs for NaN-initialized data. This patch replaces the array update with a loop excluding the unused halo values.
The initial pass_var timer was being initialized after it was used in the split RK2 timestep solver, which was causing it to be ignored in 0-initialized integers, and raising errors during integer initialization tests. This patch moves the initialization to earlier in the function, which resolves this error.
|
Gaea regression testing: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/9355 |
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1026 +/- ##
============================================
- Coverage 43.2% 41.49% -1.72%
============================================
Files 212 212
Lines 62298 62323 +25
============================================
- Hits 26918 25858 -1060
- Misses 35380 36465 +1085
Continue to review full report at Codecov.
|
adcroft
approved these changes
Nov 12, 2019
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.
This PR fixes several initialization errors detected with NaN-initialization of real variables. Most are due to invalid evaluation of quantities inside function calls, and invalid "short-circuit"
A & Blogic tests, whereBis undefined ifAis.false..Summary below of fixes:
81f3a58 (marshall/nan_fixes, nan_fixes) RK2 split pass_init clock timer initialization
0949df6 Merge branch 'dev/gfdl' into nan_merge
eca876e Merge branch 'dev/gfdl' into nan_fixes
2980893 Merge branch 'dev/gfdl' into nan_fixes
54fc734 Mixedlayer_restrat density update
f607a45 MOM_set_viscosity logic split
fa97128 MOM_entrain_diffusive logical nesting
2518c36 Initialize surface forcing adjustments to zero
b2e2511 advect_x logical test split
a016e3b Neutral diffusion unit testing tolerance set
aec5eb4 opacity: netPen explicit summation
0eaeb81 Wave speed diag: explicit argument calcs
ec851ed Extend z_top and z_btm density calculation in diag