Skip to content
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

(+)Better units documentation in MOM_tracer_hor_diff and 4 other modules #280

Merged
merged 2 commits into from
Dec 23, 2022

Conversation

Hallberg-NOAA
Copy link
Member

This commit includes minor cleanup of the units documentation in several of the process parameterization routines. These changes include:

  • Change the units of the (apparently unused) optional read_khdt_[xy] arguments to tracer_hordiff from [m2] to [L2 ~> m2]

  • Correct a tiny number used for safety in the denominator of a normalization factor based on the (nondimensional) masks from H_subroundoff to a tiny nondimensional number, giving an expression that is dimensionally consistent. Without this change, the diagnostic "KHTR_h" will vary with large enough negative values of H_RESCALE_POWER.

  • Replace a local variable in bulkmixedlayer_init with units of time with distinct get_param calls setting the default for BUFFER_LAY_DETRAIN_TIME depending on which buffer layer detrainment scheme is used.

  • Correct the dimensions of the "land_mask" diagnostic internal_tides_init from "logical" to "nondim", because it is a real multiplicable mask, not a boolean logical mask.

  • Eliminate the internal variable L2_to_Z2 in the calculate_projected_state routine in MOM_kappa_shear which had been described incorrectly and ultimately was not helpful.

  • Corrected the allocated size declaration for three 3-d v-face arrays

  • Add comments highlighting a non-stride-1 loop structure for openMP threading in tracer_epipycnal_ML_diff that required the addition of 3 extra 3-d arrays and might be a performance liability in non-openMP configurations.

  • Extensive modification of the comments describing the variables in MOM_tracer_hordiff to clearly indicate the units of each real variable and similarly of one variable in MOM_geothermal.

All answers are bitwise identical unless very large rescaling is done for thickness units, and the only change in model output is to the documented units of one diagnostic that is not used often, and to the diagnostic "KHTR_h" when large enough negative values of H_RESCALE_POWER are used.

  This commit includes minor cleanup of the units documentation in several of
the process parameterization routines.  These changes include:

- Change the units of the (apparently unused) optional read_khdt_[xy] arguments to
  tracer_hordiff from [m2] to [L2 ~> m2]

- Correct a tiny number used for safety in the denominator of a normalization
  factor based on the (nondimensional) masks from H_subroundoff to a tiny
  nondimensional number, giving an expression that is dimensionally consistent.
  Without this change, the diagnostic "KHTR_h" will vary with large enough
  negative values of H_RESCALE_POWER.

- Replace a local variable in  bulkmixedlayer_init with units of time with
  distinct get_param calls setting the default for BUFFER_LAY_DETRAIN_TIME
  depending on which buffer layer detrainment scheme is used.

- Correct the dimensions of the "land_mask" diagnostic internal_tides_init
  from "logical" to "nondim", because it is a real multiplicable mask, not a
  boolean logical mask.

- Eliminate the internal variable L2_to_Z2 in the calculate_projected_state
  routine in MOM_kappa_shear which had been described incorrectly and ultimately
  was not helpful.

- Corrected the allocated size declaration for three 3-d v-face arrays

- Add comments highlighting a non-stride-1 loop structure for openMP threading
  in tracer_epipycnal_ML_diff that required the addition of 3 extra 3-d arrays
  and might be a performance liability in non-openMP configurations.

- Extensive modification of the comments describing the variables in
  MOM_tracer_hordiff to clearly indicate the units of each real variable and
  similarly of one variable in MOM_geothermal.

All answers are bitwise identical unless very large rescaling is done for
thickness units, and the only change in model output is to the documented units of
one diagnostic that is not used often, and to the diagnostic "KHTR_h" when large
enough negative values of H_RESCALE_POWER are used.
@codecov
Copy link

codecov bot commented Dec 17, 2022

Codecov Report

Merging #280 (8f37782) into dev/gfdl (747eeff) will increase coverage by 0.00%.
The diff coverage is 77.41%.

@@            Coverage Diff            @@
##           dev/gfdl     #280   +/-   ##
=========================================
  Coverage     37.08%   37.09%           
=========================================
  Files           263      263           
  Lines         73586    73588    +2     
  Branches      13718    13719    +1     
=========================================
+ Hits          27292    27294    +2     
+ Misses        41259    41258    -1     
- Partials       5035     5036    +1     
Impacted Files Coverage Δ
...c/parameterizations/lateral/MOM_internal_tides.F90 0.00% <0.00%> (ø)
src/parameterizations/vertical/MOM_geothermal.F90 28.92% <0.00%> (ø)
...arameterizations/vertical/MOM_bulk_mixed_layer.F90 48.19% <33.33%> (-0.11%) ⬇️
src/tracer/MOM_tracer_hor_diff.F90 76.54% <90.90%> (+0.06%) ⬆️
src/parameterizations/vertical/MOM_kappa_shear.F90 74.07% <100.00%> (-0.04%) ⬇️
src/framework/MOM_document.F90 74.33% <0.00%> (+0.44%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@marshallward
Copy link
Member

@marshallward marshallward merged commit 3794b83 into NOAA-GFDL:dev/gfdl Dec 23, 2022
@Hallberg-NOAA Hallberg-NOAA deleted the minor_units_cleanup branch February 2, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants