forked from mom-ocean/MOM6
-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
*MOM-ocean/Main to NOAA-GFDL/main (2024-08-24) (Parentheses for FMA symmetry) #716
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 parentheses to expressions taking the squares of the wind stress components in 70 lines in 7 files so that these expressions will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
Added parentheses to 9 expressions like `curv_3 = h_W(i) + h_E(i) - 2.0*h(i)` in PPM_limit_pos, zonal_flux_layer, zonal_flux_thickness, merid_flux_layer and merid_flux_thickness, changing them to `curv_3 = (h_W(i) + h_E(i)) - 2.0*h(i)`. This change enforces the order of arithmetic that is required to give rotational symmetry, but it also is the order that the Intel, GNU, and Nvidia compliers were all already using in these expressions. Moreover, had the order of arithmetic ever been anything else, this would have led to failures in our rotational consistency and redundant point consistency testing, and almost certainly would have been detected before. However, by adding these parentheses, there is a remote chance that the addition of these parentheses could change answers for some compiler or compiler settings we have never tested before. This change should not impact any FMA-enabled calculations. All answers are bitwise identical in the MOM6-examples regression suite as run on Gaea.
Added parentheses to 16 expressions setting the grad_gradient arrays with oblique_grad open boundary conditions and setting cff_new with all kinds of oblique boundary conditions so that they will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers with certain types of open boundary conditions could change with FMAs.
Added parentheses to 150 lines in the 5 generic density integral routines (int_density_dz_generic_pcm, int_density_dz_generic_plm, int_density_dz_generic_ppm, int_spec_vol_dp_generic_pcm and int_spec_vol_dp_generic_plm) in the MOM_density_integrals module so that they will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
Added parentheses to 140 lines in 8 int_density_dz and int_spec_vol_dp routines for the linear, Wright, Wright_full and Wright_red equations of state so that they will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
Removed recently added parentheses around expressions like '+ (hL*hR)' in 110 lines in MOM_density_integrals and 4 equation of state module to reflect that these parentheses are not necessary for rotational symmetry in FMAs. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
Added parentheses to 4 lines in PressureForce_Mont_nonBouss and PressureForce_Mont_Bouss so that they will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs in cases that use the Montgomery potential form of the pressure gradient accelerations.
Added parentheses to 4 lines in calc_isoneutral_slopes so that they will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
Added parentheses to 2 lines in MOM_calc_varT so that they will be rotationally invariant when fused-multiply-adds are enabled. In this case, FMAs can still be applied to the impacted lines, exploiting that the masks are always 0 or 1. Also added parentheses to 2 other lines used to generate the stochastic pattern for rotational symmetry with FMAs. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
Added parentheses to the calculation of the diffusive temperature changes in tracer_hordiff so that it will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
Added parentheses to the calculation of the iceberg contribution to the fractional area of ice shelves in iceberg_forces so that it will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs enabled in cases with tabular icebergs.
Added parentheses to the calculation of the Stokes-drift Coriolis velocity increments in CoriolisStokes so that it will be rotationally invariant when fused-multiply-adds are enabled. All answers are bitwise identical because CoriolisStokes is still under development and is never called, with a fatal error occurring if anyone tries to use it. Also added parentheses to two expressions calculating the magnitude of the Stokes velocity in get_Langmuir_Number. Answers could change for some cases that use Langmuir turbulence parameterizations with FMAs enabled.
Added the new element Coriolis2Bu to the ocean_grid_type and the dyn_horgrid_type to hold the square of the Coriolis parameter, and use this array in 10 routines (including btstep, set_dtbt, calculate_diagnostic_fields, VarMix_init, propagate_int_tide, Calculate_kappa_shear, Calc_kappa_shear_vertex and add_MLrad_diffusivity) that had been calculating and averaging the square of the Coriolis parameter. This could change some answers with FMAs enabled because the compilers were previously free to split up some of the squares when averaging the squared Coriolis parameter, but without FMAs all answers are bitwise identical. This commit does add a new element to two transparent types.
Added parentheses to 17 expressions in thickness_diffuse_full, thickness_diffuse and thickness_diffuse_init to give rotationally consistent solutions when fused-multiply-adds are enabled. One comment was also added to note that the calculation of PE_release_h is does not exhibit rotational symmetry when MEKE_GM_SRC_ALT is set to true. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 2 expressions in the Zanna_Bolton code and rearranged another line so that the u- and v-discretizations introduce terms in the same order so that the Zanna_Bolton code will exhibit rotationally consistent solutions when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs enabled in cases that use the Zanna-Bolton parameterization.
Added parentheses to 19 expressions in the MOM_internal_tides propagation code to exhibit rotationally consistent solutions when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled in models that use the ray-tracing based internal tides code.
Added parentheses to 10 expressions in find_uv_at_h to exhibit rotationally consistent solutions and treat the velocities at both edges of a tracer cell equivalently when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 19 expressions in set_viscous_ML, set_u_at_v and set_v_at_u to treat the velocities at both edges of a tracer cell equivalently when fused-multiply-adds are enabled, and thereby to exhibit exhibit rotationally consistent solutions. Also swapped the order of the u- and v-components in the u-point calculation of Uh2 to mirror the order of the corresponging v-point calculation for the same purpose. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added mathematically equivalent rearrangements of the code in calc_kappa_shear_vertex that interpolates velocities, temperatures and salinities to the vertices to expose the mask variables while ensuring that the other multiplications occur within parentheses so that they will exhibit rotational symmetry when fused-multiply-adds are enabled. FMAs can still occur, but it will be multiplication by the 0-or-1 masks that are fused with an addition. Also added parentheses to 3 expressions calculating the squared shear in calculate_projected_state for rotational symmetry with FMAs. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 4 expressions in add_drag_diffusivity, set_BBL_TKE and add_LOTW_BBL_diffusivity setting the bottom-drag contributions to TKE and friction velocity so that they will exhibit rotationally consistent solutions when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 20 expressions in CorAdCalc and one in gradKE to exhibit rotationally consistent solutions when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 18 expressions in btstep, and one more each in set_dtbt and barotropic_init to exhibit rotationally consistent solutions when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 19 expressions in 5 routines (calc_Visbeck_coeffs_old, calc_Eady_growth_rate_2D, calc_slope_functions_using_just_e, calc_QG_Leith_viscosity VarMix_init) in MOM_lateral_mixing_coeffs.F90 to give rotationally consistent solutions when fused-multiply-adds are enabled. Also reordered terms in a sum in the calculation of beta_dx2_u to mirror that of beta_dx2_v, also for rotational symmetry with FMAs. All answers are bitwise identical in cases without FMAs, but answers could change for some parameter settings when FMAs are enabled.
Added parentheses to 40 expressions horizontal_viscosity and another 14 expressions in in hor_visc_init and 3 more in align_aniso_tensor_to_grid to give rotationally consistent solutions when fused-multiply-adds are enabled. Also swapped the order of two terms in the expression for Del2u to mirror the order of the corresponding terms in Del2v for rotational symmetry with FMAs. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 20 sums of squares of x- and y- distances or velocity components used for initialization in 8 modules to give rotationally consistent solutions when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 29 sums of squares of velocity or other vector components used in parameterizations in 9 modules to give rotationally consistent solutions when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 9 diagnostics of Coriolis accelerations or expressions used in the kinetic energy budgets to give rotationally consistent solutions when fused-multiply-adds are enabled. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses to 4 tracer edge value calculations used with PPM tracer advection to give rotationally consistent solutions when fused-multiply-adds are enabled. Although these lines may not appear to need parentheses, some compliers appear to be putting these expressions directly into others, where the direction of the flow seems to determine which multiplications are incorporated into FMAs. All answers are bitwise identical in cases without FMAs, but answers could change when FMAs are enabled.
Added parentheses around the full expressions for intz and intp in 10 more lines in 4 generic density integral routines (int_density_dz_generic_pcm, int_density_dz_generic_ppm, int_spec_vol_dp_generic_pcm and int_spec_vol_dp_generic_plm) in the MOM_density_integrals module so that non-Boussinesq cases will be rotationally invariant when fused-multiply-adds are enabled. Although this might not seem to do anything, these parentheses do matter if these expressions are in-lined in the sums where they are used a few lines later. The analogous parentheses had previously been added to int_density_dz_generic_plm. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
Added parentheses to prevent FMAs in 4 expressions in end_value_h4 that rely on exact vertical symmetry in order to get the cancellations that are necessary to pass the vertical remapping unit testing. Before this change, the MOM6 unit-testing was failing when FMAs are enabled, but with it the unit-testing is passing even when FMAs are enabled. All answers are bitwise identical in cases without FMAs, but answers could change with FMAs.
(*)Add parentheses for FMA rotational symmetry
Hallberg-NOAA
added
enhancement
New feature or request
answer-changing
A change in results (actual or potential)
labels
Aug 24, 2024
Hallberg-NOAA
changed the title
*Main to dev/gfdl (2024-08-24) (Parentheses for FMA symmetry)
*MOM-ocean/Main to NOAA-GFDL/main (2024-08-24) (Parentheses for FMA symmetry)
Aug 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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 is a merge of the recent updates to main that came from https://github.com/Hallberg-NOAA/MOM6/tree/FMA_rotational_symmetry_main. These changes give rotational symmetry when Fused-Multiply-Adds (FMAs) are enabled, but they are bitwise identical when then are not. The full description from the original PR to MOM-ocean/main follows:
This series of 30 commits adds parentheses in 54 files throughout the MOM6 code base, or (in a few cases) revises the order of sums in expressions for various u- and v-component calculations, so that all answers exhibit rotational symmetry when fused-multiply-adds (FMAs) are enabled and appropriate parameter settings are chosen.
As a part of these changes, there is a new element ocean_grid_type%Coriolis2Bu with the square of the Coriolis parameter in the widely used transparent ocean grid type.
In order to obtain rotational symmetry (with or without FMAs) for all cases in the NOAA-GFDL MOM6-examples regression suite where FMS supports grid rotation (plus non-Boussinesq versions of these test cases and some extra examples to reflect parameterizations used in CESM prototypes), the following parameter settings may be needed:
#override DEFAULT_ANSWER_DATE = 99991231 ! This gives rotational symmetry if it is higher than 20240701
#override HOR_DIFF_ANSWER_DATE = 99991231 ! This must be 20240331 or higher for rotational symmetry.
#override NDIFF_ANSWER_DATE = 99991231 ! This must be 20240331 or higher for rotational symmetry.
#override WAVE_INTERFACE_ANSWER_DATE = 99991231 ! This must be 20230103 or higher for rotational symmetry.
#override MEKE_GM_SRC_ANSWER_DATE = 99991231 ! This must be 20240601 or higher for rotational symmetry.
#override BT_USE_OLD_CORIOLIS_BRACKET_BUG = False
#override USE_GM_WORK_BUG = False
#override MEKE_GM_SRC_ALT_SLOPE_BUG = False
#override USE_OCMIP2_CFC = False ! The FMS routine atmos_ocn_coupler_flux() does not support grid rotation yet.
Note that FMS does not currently support an eastern, western or southern tripolar grid, so global test cases with a northern tripolar grid can not be tested for rotational symmetry. However, the Baltic test cases have been used to verify the rotational symmetry of all of the parameterizations that are used in global models.
Note also that automated grid rotation testing is not supported yet for all types of open boundary conditions or for velocities with ALE sponges.
All answers are bitwise identical when FMAs are not enabled, but answers do change when FMAs are enabled. The only change in public interfaces is the addition of a new squared Coriolis parameter element in the transparent ocean grid type. The list of commits in this PR include:
fd82861e3 ()Add parentheses in end_value_h4 for FMAs
ffa766b45 ()More parentheses in density integrals for FMAs
e810ac537 ()Parenthesize tracer_advect PPM edge values
182223c50 ()Parenthesize diagnostics for FMAs
44f1130fc ()Parenthesize parameterization squares for FMAs
fc2af28ed ()Parenthesize initialization squares for FMAs
ffef92f7f ()Parenthesize MOM_hor_visc for FMAs
6216fa1f5 ()Parenthesize MOM_lateral_mixing_coeffs for FMAs
46e8b661e ()Parenthesize MOM_barotropic for FMAs
64b851c4e ()Parenthesize CorAdCalc for FMAs
f0c52ddb1 ()Parenthesize MOM_set_diffusivity for FMAs
0b50a155a ()Rearrange calc_kappa_shear_vertex for FMAs
c0bef189a ()Parenthesize set_viscous_ML for FMAs
ebf02a97a ()Parenthesize find_uv_at_h for FMAs
654cd4aab ()Parenthesize MOM_internal_tides for FMAs
03dc6f9a5 ()Parenthesize Zanna_Bolton for FMAs
49419f732 ()Parenthesize thickness_diffuse for FMAs
56d053a04 +()Add and use G%Coriolis2Bu
5398e6fdf ()Parenthesize CoriolisStokes and LA_Stk for FMAs
b2beab215 ()Parenthesize iceberg_forces for FMAs
c344a117f ()Parenthesize tracer_hordiff for FMAs
ce559ce64 ()Parenthesize MOM_calc_varT for FMAs
307a4e2e5 ()Parenthesize calc_isoneutral_slopes for FMAs
99fd95796 ()Parenthesize PressureForce_Montgomery for FMAs
8066a3da0 ()Simplify density integral parentheses
24091ccf0 ()Add parentheses to 4 EOS int_density routines
9172cd576 ()Add parentheses for density_integrals with FMAs
4f710efb0 ()Add parentheses for oblique OBCs with FMAs
f0e61f30c ()Parenthesize continuity_PPM curv_3 expressions
6628dbff5 ()Parenthesize squares of wind stresses for FMAs