Fix units for MEKE advection #1039
Conversation
- Code was passing dimensional testing because a conversion factor was included to obtain the right units for the advective flux terms. The origin of the unit problem was in the rate of inter-column exchange (baroHu and baroHv) which were in units of [H L2] and should have been in units of [R Z L2]: - [H L2 ~> m3 or kg] so differs between Boussinesq and non-Boussinesq; - [R Z L2 ~> kg] for both modes. - This commit moves the conversion to the summation for the barotropic exchange, removes the previous scaling factors, and updates the relevant comments. - Closes #1036
- Removed commented code snippet that was out-of-date w.r.t. to the current code and completely misleading.
- Cleaned up/added comments annotating units.
|
Pipeline test at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/9449 |
| @@ -266,7 +265,7 @@ subroutine step_forward_MEKE(MEKE, h, SN_u, SN_v, visc, dt, G, GV, US, CS, hu, h | |||
| enddo ; enddo | |||
| do i=is-1,ie+1 | |||
| I_mass(i,j) = 0.0 | |||
| if (mass(i,j) > 0.0) I_mass(i,j) = 1.0 / mass(i,j) | |||
| if (mass(i,j) > 0.0) I_mass(i,j) = 1.0 / mass(i,j) ! [m2 kg-1] | |||
There was a problem hiding this comment.
The correct units in the comment here should be '[R-1 Z-1 ~> m2 kg-1]'
| Inv_K4_max = 64.0 * sdt * ((G%dx_Cv(i,J)*G%IdyCv(i,J)) * max(G%IareaT(i,j), G%IareaT(i,j+1)))**2 | ||
| if (K4_here*Inv_K4_max > 0.3) K4_here = 0.3 / Inv_K4_max | ||
|
|
||
| ! Here the units of MEKE_vflux are [kg m-2 L4 T-3]. |
There was a problem hiding this comment.
The correct units here are '[R Z L4 T-3 ~> kg m2 s-3]'
| !### I think that for dimensional consistency, this should be: | ||
| ! advFac = GV%H_to_RZ * CS%MEKE_advection_factor / sdt | ||
| advFac = US%kg_m3_to_R*GV%H_to_Z * CS%MEKE_advection_factor / dt | ||
| advFac = CS%MEKE_advection_factor / sdt ! [T-1] |
There was a problem hiding this comment.
The units here should have the converted units, as '[T-1 ~> s-1]'
| MEKE_vflux(i,J) = ((K4_here * (G%dx_Cv(i,J)*G%IdyCv(i,J))) * & | ||
| ((2.0*mass(i,j)*mass(i,j+1)) / ((mass(i,j)+mass(i,j+1)) + mass_neglect)) ) * & | ||
| (del2MEKE(i,j+1) - del2MEKE(i,j)) | ||
| enddo ; enddo | ||
| ! Store tendency arising from the bi-harmonic in del4MEKE | ||
| ! Store change in MEKE arising from the bi-harmonic in del4MEKE [L2 T-2]. |
There was a problem hiding this comment.
The units here should have the converted MKS units, as '[L2 T-2 ~> m2 s-2]'
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1039 +/- ##
===========================================
Coverage ? 45.12%
===========================================
Files ? 212
Lines ? 62509
Branches ? 0
===========================================
Hits ? 28206
Misses ? 34303
Partials ? 0
Continue to review full report at Codecov.
|
|
Yes - these are all because of a bad merge. |
Three commits: