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

Change units of slope returned from calc_isoneutral_slopes() to "Z L-1" #1351

Merged
merged 2 commits into from
Mar 12, 2021
Merged

Change units of slope returned from calc_isoneutral_slopes() to "Z L-1" #1351

merged 2 commits into from
Mar 12, 2021

Conversation

adcroft
Copy link
Collaborator

@adcroft adcroft commented Mar 12, 2021

This addresses the units of interface slope, or isoneutral slope, used for parameterizing thickness transport. I came across this while fixing the Eady growth rate calculation which will come next and depends on this PR but for the sake of smaller PRs am submitting them as a dependent sequence.

From the commit message:

  • Units of isoneutral or interface slope were recorded as "nondim". While true in SI units, not so for MOM6 units. MOM6 distinguishes between units of length in the vertical (Z) and horizontal (L) the slopes should have units of "Z L-1 ~> nondim".
  • This has consequences for other variables in calc_isoneutral_slopes().
    • An internal constant, G_Rho0, was defined differently from elsewhere in the code. "g" has units of "L2 Z-1 T-2 ~ m s-2" because it is the vertical component of the gradient of geopotential in "L2 T-2 ~ m2 s-2". Everywhere else G_Rho0 = g_Earth/Rho0 but in this routine it was different in order render N2 (the Brunt-Vaisala frequency) in units of "T-2" (s-2).
    • N2 is a quantity associated with dispersion relations and defined N2 = - g/Rho0 d/dz rho and either way acquires units of "L2 Z-2 T-2" and not just "T-2". In SI units L2 Z-2 = 1. So I have also changed the units of N2 in this, and connected, modules.
  • The changes also propagate to MOM_lateral_mixing_coeffs.F90 and MOM_thickness_diffuse.F90.
  • Changing the definition of G_Rho0 in calc_isoneutral_slopes(), and its units to "L2 Z-1 T-2", the slope and N2 calculations then require many less inline conversions. Many of the one-line changes in this commit remove factors like US%Z_to_L. There is one exception:
    • In the calculation of slope, we use in the denominator a mostly non-vanishing replacement for d/dz rho, the magnitude of grad rho from mag_grad2 = ( d/dx rho )^2 + ( d/dz rho )^2. In code this had mag_grad2 = drdy**2 + (L_to_Z*drdz)**2 which is mixing gradients in the horizontal and vertical. The result should be in "R2 Z-2" so now mag_grad2 = (Z_to_L*drdy)**2 + drdz**2
  • A few diagnostics needed new, or changed, conversion factors.
  • One run-time parameter needed a conversion parameter.
  • For the most part this commit moves inline conversions of units to the I/O stage, which is an indicator that it is the right thing to do.

@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #1351 (bb53e88) into dev/gfdl (4255ada) will not change coverage.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff            @@
##           dev/gfdl    #1351   +/-   ##
=========================================
  Coverage     45.83%   45.83%           
=========================================
  Files           234      234           
  Lines         72663    72663           
=========================================
  Hits          33302    33302           
  Misses        39361    39361           
Impacted Files Coverage Δ
...arameterizations/lateral/MOM_thickness_diffuse.F90 64.23% <71.42%> (ø)
src/core/MOM_isopycnal_slopes.F90 87.38% <77.77%> (ø)
...eterizations/lateral/MOM_lateral_mixing_coeffs.F90 59.76% <100.00%> (ø)

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 4255ada...ab241b6. Read the comment docs.

real, dimension(SZIB_(G),SZJ_(G),SZK_(GV)+1), &
optional, intent(inout) :: N2_u !< Brunt-Vaisala frequency squared at
!! interfaces between u-points [T-2 ~> s-2]
real, dimension(SZI_(G),SZJB_(G),SZK_(GV)+1), &
optional, intent(inout) :: N2_v !< Brunt-Vaisala frequency squared at
!! interfaces between u-points [T-2 ~> s-2]
!! interfaces between v-points [L2 Z-2 T-2 ~> s-2]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with this change, but for consistency there also has to be an equivalent change to the documented units of N2_u 3 lines above this change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good spot - this was due to conflicts when I cherry-picked this out of my other branch

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did a force push to avoid new commits.

Copy link
Collaborator

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

I have inspected these changes, and I wholeheartedly agree that they are the right and parsimonious thing to do here. I will happily approve these changes after one very minor oversight is corrected on line 44 of MOM_isopycnal_slopes.F90.

adcroft and others added 2 commits March 12, 2021 15:01
- Units of isoneutral or interface slope were recorded as "nondim". While
  true in SI units, not so for MOM6 units. MOM6 distinguishes between units
  of length in the vertical (Z) and horizontal (L) the slopes should have
  units of "Z L-1 ~> nondim".
- This has consequences for other variables in calc_isoneutral_slopes().
  - An internal constant, G_Rho0, was defined differently from elsewhere in
    the code. "g" has units of "L2 Z-1 T-2 ~ m s-2" because it is the
    vertical component of the gradient of geopotential in "L2 T-2 ~ m2 s-2".
    Everywhere else `G_Rho0 = g_Earth/Rho0` but in this routine it was
    different in order render N2 (the Brunt-Vaisala frequency) in units of
    "T-2" (s-2).
  - N2 is a quantity associated with dispersion relations and defined
    N2 = - g/Rho0 d/dz rho and either way acquires units of "L2 Z-2 T-2"
    and not just "T-2". In SI units L2 Z-2 = 1. So I have also changed
    the units of N2 in this, and connected, modules.
- The changes also propagate to MOM_lateral_mixing_coeffs.F90 and
  MOM_thickness_diffuse.F90.
- Changing the definition of G_Rho0 in calc_isoneutral_slopes(), and
  its units to "L2 Z-1 T-2", the slope and N2 calculations then require
  many less inline conversions. Many of the one-line changes in this commit
  remove factors like US%Z_to_L.There is one exception:
  - In the calculation of slope, we use in the denominator a mostly
    non-vanishing replacement for d/dz rho, the magnitude of grad rho from
    mag_grad2 = ( d/dx rho )^2 + ( d/dz rho )^2. In code this had
    `mag_grad2 = drdy**2 + (L_to_Z*drdz)**2` since this is mixing
    gradients in the horizontal and vertical. The result should be
    in "R2 Z-2" so now `mag_grad2 = (Z_to_L*drdy)**2 + drdz**2`
- A few diagnostics needed new, or changed, conversion factors.
- One run-time parameter needed a conversion parameter.
- For the most part this commit moves inline conversions of units to
  the I/O stage, which is an indicator that it is the right thing to do.
@marshallward
Copy link
Collaborator

marshallward commented Mar 12, 2021

@marshallward marshallward merged commit a6f680c into mom-ocean:dev/gfdl Mar 12, 2021
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.

3 participants