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

+Argument cleanup in vertical parameterization code #1525

Merged
merged 2 commits into from
Oct 20, 2021

Conversation

Hallberg-NOAA
Copy link
Collaborator

Cleaned up 27 falsely optional or unused arguments in the vertical
parameterization code, and related changes. This includes:

  • Eliminating the symmetrize arguments to set_viscous_BBL and set_viscous_ML,
    which are now effectively always true.
  • Making the Waves and OBC pointer arguments mandatory in several routines
    where they were always being supplied. These are pointers, so the test of
    whether they should be used can be based on whether they are associated.
  • Adding error messages about unassociated Waves types that would be used.
  • Eliminating the unused Waves argument to KPP_init.
  • Eliminating unused arguments that energetic_PBL inherited from the bulk mixed
    layer code, and simplified some disabled debugging code.
  • Making the optics argument to opacity_end mandatory.
  • Making the h argument to get_Langmuir_Number mandatory and rearranged the
    arguments to this routine to put the optional arguments last, following the
    practice elsewhere in the MOM6 code. Also standardized the case of several
    variables in the MOM_wave_interface.F90 code to facilitate searches.
  • Eliminating the unused US argument to CoriolisStokes.
    All answers are bitwise identical, and no output is changed.

  Cleaned up 27 falsely optional or unused arguments in the vertical
parameterization code, and related changes.  This includes:
 - Eliminating the symmetrize arguments to set_viscous_BBL and set_viscous_ML,
   which are now effectively always true.
 - Making the Waves and OBC pointer arguments mandatory in several routines
   where they were always being supplied.  These are pointers, so the test of
   whether they should be used can be based on whether they are associated.
 - Adding error messages about unassociated Waves types that would be used.
 - Eliminating the unused Waves argument to KPP_init.
 - Eliminating unused arguments that energetic_PBL inherited from the bulk mixed
   layer code, and simplified some disabled debugging code.
 - Making the optics argument to opacity_end mandatory.
 - Making the h argument to get_Langmuir_Number mandatory and rearranged the
   arguments to this routine to put the optional arguments last, following the
   practice elsewhere in the MOM6 code.  Also standardized the case of several
   variables in the MOM_wave_interface.F90 code to facilitate searches.
 - Eliminating the unused US argument to CoriolisStokes.
All answers are bitwise identical, and no output is changed.
@codecov
Copy link

codecov bot commented Oct 17, 2021

Codecov Report

Merging #1525 (abff7bc) into dev/gfdl (c62258c) will decrease coverage by 0.00%.
The diff coverage is 42.10%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl    #1525      +/-   ##
============================================
- Coverage     29.08%   29.08%   -0.01%     
============================================
  Files           239      239              
  Lines         71611    71586      -25     
============================================
- Hits          20830    20821       -9     
+ Misses        50781    50765      -16     
Impacted Files Coverage Δ
...parameterizations/vertical/MOM_diabatic_driver.F90 39.23% <ø> (ø)
src/user/MOM_wave_interface.F90 0.83% <0.00%> (ø)
src/parameterizations/vertical/MOM_CVMix_KPP.F90 0.99% <11.11%> (-0.01%) ⬇️
...c/parameterizations/vertical/MOM_energetic_PBL.F90 46.02% <60.00%> (+0.41%) ⬆️
src/core/MOM.F90 59.01% <66.66%> (ø)
src/core/MOM_dynamics_split_RK2.F90 61.08% <100.00%> (ø)
src/parameterizations/vertical/MOM_opacity.F90 28.60% <100.00%> (ø)
...parameterizations/vertical/MOM_set_diffusivity.F90 32.81% <100.00%> (ø)
...c/parameterizations/vertical/MOM_set_viscosity.F90 47.36% <100.00%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c62258c...abff7bc. Read the comment docs.

@marshallward
Copy link
Collaborator

marshallward commented Oct 20, 2021

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/13865 ✔️

MASKING_DEPTH description has been modified. Nevermind, I think this was an out-of-date branch.

Copy link
Collaborator

@marshallward marshallward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing some of the present() could be a problem with the current effort to remove associate() blocks, since sometimes I am counting on the present() to justify the associate() removal.

I think these are all OK; for example, only potential issue is present(optics), but individual allocated() checks save the day. But something we should monitor as optional arguments are shifted to mandatory ones.

@marshallward marshallward merged commit 037af8e into mom-ocean:dev/gfdl Oct 20, 2021
@Hallberg-NOAA Hallberg-NOAA deleted the vert_param_args branch November 27, 2021 11:13
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