Update MOM_mixed_layer_restrat.F90 #568

marshallward · 2024-02-08T17:36:54Z

Adding and calculating diagnostic front length scale (lf_bodner) to mixed layer restratification code

marshallward · 2024-02-08T17:38:32Z

I think this addresses the issues raised in #532 and #542. Authorship of @amoebaliz was preserved, with @kshedstrom and myself added as co-authors.

codecov · 2024-02-08T17:47:17Z

Codecov Report

Attention: Patch coverage is 63.15789% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 37.20%. Comparing base (3f7465a) to head (7032df3).

Files	Patch %	Lines
...ameterizations/lateral/MOM_mixed_layer_restrat.F90	63.15%	4 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           dev/gfdl     #568   +/-   ##
=========================================
  Coverage     37.20%   37.20%           
=========================================
  Files           271      271           
  Lines         80457    80473   +16     
  Branches      15002    15005    +3     
=========================================
+ Hits          29932    29943   +11     
- Misses        44958    44960    +2     
- Partials       5567     5570    +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hallberg-NOAA

This revised PR addresses all of the concerns that I had with its predecessors (PR #532 and #542).

marshallward · 2024-02-09T15:28:37Z

I seem to have changed something which let u_star3 become NaN, this needs to be fixed.

marshallward · 2024-02-09T17:28:04Z

The splitting of the loop to accommodate the new ANSWER_DATE caused u_star3 and w_star3 to be unassigned and to lose their Z3/T3 dimensional corrections. These have now been fixed. (They will probably be squashed, assuming everything looks correct.)

I removed most of the explicit dimension scalings in the diag formula, which should no longer be needed since the new cuberoot() preserves dimensions and we don't need to accomodate it anymore. But I may have bungled up some Z's and H's here and it may need some further review.

amoebaliz · 2024-02-09T17:51:18Z

Thanks Marshall!

…

On Fri, Feb 9, 2024 at 12:28 PM Marshall Ward ***@***.***> wrote: The splitting of the loop to accommodate the new ANSWER_DATE caused u_star3 and w_star3 to be unassigned and to lose their Z3/T3 dimensional corrections. These have now been fixed. (They will probably be squashed, assuming everything looks correct.) I removed most of the explicit dimension scalings in the diag formula, which should no longer be needed since the new cuberoot() preserves dimensions and we don't need to accomodate it anymore. But I may have bungled up some Z's and H's here and it may need some further review. — Reply to this email directly, view it on GitHub <#568 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD6PXJHWORQ2HSVUB3PQXVTYSZMDBAVCNFSM6AAAAABDAE4SYCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZWGMYTGMRUGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

marshallward · 2024-02-09T18:52:37Z

I am kind of baffled about the dimension error here. There's a Z^3 missing somewhere, but everything else is passing, and I don't know how that is possible.

I'm also unable to replicate these dimensional errors when I run locally.

marshallward · 2024-02-09T19:24:54Z

Dimensions of Z2/H seemed to have fixed it.

I am still confused about the units of ustar, but in the context of this module I can see that it's consistent.

marshallward · 2024-02-13T19:32:49Z

This need a new review from @Hallberg-NOAA when he gets back from leave.

src/parameterizations/lateral/MOM_mixed_layer_restrat.F90

Hallberg-NOAA · 2024-02-17T16:48:59Z

src/parameterizations/lateral/MOM_mixed_layer_restrat.F90

+
+      lf_bodner_diag(i,j) = &
+          0.25 * cuberoot(CS%mstar * u_star3 + CS%nstar * w_star3)**2 &
+            / (f2_h * max(little_h(i,j), GV%Angstrom_H))


In order to convert this to units of [L ~> m], the denominator should be multiplied by (US%L_to_Z * GV%H_to_RZ* tv%SpV_avg(i,j,1)) when in fully non-Boussinesq mode or in Boussinesq mode the denominator should be multiplied by (GV%H_to_Z*US%L_to_Z). (Compare this with the expressions that set wpup at lines 943 and 968.) Were we to make this change here, the conversion factor at line 1814 would need to be changed to US%L_to_m, while the units of lf_bodner_diag on line 811 would stay as they are.

I have implemented this suggestion.

Hallberg-NOAA

This PR should be changed in one of two ways - either to document the units of lf_bodner_diag consistently with how the code is written, or to revise lf_bodner_diag so that it is in the more natural units of [L ~> m]. I would prefer the latter, as it gives a diagnostic that is much more natural in non-Boussinesq mode (i.e., it avoids using the Boussinesq reference density). There are two separate comments attached to specific lines describing each of the two (mutually exclusive) options.

marshallward · 2024-02-20T18:23:27Z

I agree with @Hallberg-NOAA's second suggestion: if the Bodner lengthscale is L then it should be saved as L.

I've made some changes based on your suggestion to move the LH Z-2 scaling back into the equation for lf_bodner. Since wpup did the same scaling, I moved the Boussinesq/non-Boussinesq forms of this scaling factor into a new variable. (It's possible some consolidation could be done here, but I left things as they are.)

I also moved the id_lfbod if-block outside of the do-loop, to help with performance.

Hallberg-NOAA · 2024-02-25T15:59:55Z

src/parameterizations/lateral/MOM_mixed_layer_restrat.F90

@@ -928,14 +937,20 @@ subroutine mixedlayer_restrat_Bodner(CS, G, GV, US, h, uhtr, vhtr, tv, forces, d
    CS%MLD_filtered_slow(i,j) = big_H(i,j)
  enddo ; enddo

+  if (allocated(tv%SpV_avg) .and. .not.(GV%Boussinesq .or. GV%semi_Boussinesq)) then
+    uflux_rescale = US%Z_to_L * GV%RZ_to_H / tv%SpV_avg(i,j,1)


In non-Boussinesq mode, uflux_rescale is a function of i and j, and it therefore either needs to be turned into a 2-d array or calculated inside of the relevant i- and j- loops.

Hallberg-NOAA · 2024-02-25T16:02:01Z

src/parameterizations/lateral/MOM_mixed_layer_restrat.F90

-                                 CS%nstar * max(0., -bflux(i,j)) * BLD(i,j) ))**2, CS%min_wstar2) * &
-                  ( US%Z_to_L * GV%RZ_to_H / tv%SpV_avg(i,j,1))
+                                 CS%nstar * max(0., -bflux(i,j)) * BLD(i,j) ))**2, CS%min_wstar2) &
+                  * uflux_rescale


Line 953 is not equivalent to the previous line 938 because the former was a function of i- and j-.

Adding and calculating diagnostic front length scale (lf_bodner) to mixed layer restratification code Co-authored-by: Kate Hedstrom <kshedstrom@alaska.edu> Co-authored-by: Marshall Ward <marshall.ward@noaa.gov>

Fix two issues which emerged from the Bodner length scale diagnostic after the cuberoot ANSWER_DATE was introduced. The split of the loop due to the cuberoot caused ustar to be assigned in a separate loop, leaving it uninitialized for the length scale diagnostics loop. It also had its dimensions removed. However, the whole point of the new cuberoot() function was to preserve its dimensions, so there was no need for the intermediate dimensional manipulation, and all dimensionality should be shifted to the registration. This patch removes the intermediate dimensions and also explicitly defines it as Z2/H rather than L.

The scaling of the Bodner lengthscale diagnostic is correctly restored to L in this patch. The Z2 H-1 constant is factored into the calculation of the lengthscale; I believe this represents a conversion from a [Z2 T-2]-like momentum flux to a [LH T-2]-like momentum flux. A generic `uflux_rescale` term was introduced to be used in both ld_bodner and wpup, to avoid a double division by SpV. The if-block for computation of ld_bodner was also moved outside of the loop, in order to facilitate vectorization.

Hallberg-NOAA

This pull request now looks correct to me.

Hallberg-NOAA · 2024-02-25T16:21:42Z

src/parameterizations/lateral/MOM_mixed_layer_restrat.F90

+
+      ! Rescale from [Z2 H-1 to L]
+      lf_bodner_diag(i,j) = lf_bodner_diag(i,j) * uflux_rescale
+    enddo ; enddo


This line has a horizontal indexing error in non-Boussinesq mode because in that case the appropriate rescaling factor is a function of (i,j).

Hallberg-NOAA · 2024-02-25T20:13:03Z

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/22420.

marshallward force-pushed the test_mle_diag_v3 branch 2 times, most recently from c945458 to cd69215 Compare February 8, 2024 17:44

Hallberg-NOAA approved these changes Feb 8, 2024

View reviewed changes

marshallward force-pushed the test_mle_diag_v3 branch from dd160e0 to 63136f1 Compare February 9, 2024 17:23

marshallward force-pushed the test_mle_diag_v3 branch from 63136f1 to 4dd2e69 Compare February 9, 2024 19:10

marshallward requested a review from Hallberg-NOAA February 13, 2024 19:33

Hallberg-NOAA reviewed Feb 17, 2024

View reviewed changes

src/parameterizations/lateral/MOM_mixed_layer_restrat.F90 Show resolved Hide resolved

Hallberg-NOAA reviewed Feb 17, 2024

View reviewed changes

Hallberg-NOAA requested changes Feb 17, 2024

View reviewed changes

marshallward force-pushed the test_mle_diag_v3 branch from d6bfdf7 to 0304fb5 Compare February 20, 2024 17:35

Hallberg-NOAA reviewed Feb 25, 2024

View reviewed changes

amoebaliz and others added 3 commits February 25, 2024 11:40

Update MOM_mixed_layer_restrat.F90

b4c39f1

Adding and calculating diagnostic front length scale (lf_bodner) to mixed layer restratification code Co-authored-by: Kate Hedstrom <kshedstrom@alaska.edu> Co-authored-by: Marshall Ward <marshall.ward@noaa.gov>

marshallward force-pushed the test_mle_diag_v3 branch 2 times, most recently from 42b0f4b to 7032df3 Compare February 25, 2024 16:41

Hallberg-NOAA approved these changes Feb 25, 2024

View reviewed changes

This was referenced Feb 25, 2024

Update MOM_mixed_layer_restrat.F90 #532

Closed

Test mle diag #542

Closed

Hallberg-NOAA merged commit f92e4ac into NOAA-GFDL:dev/gfdl Feb 25, 2024
12 checks passed

marshallward deleted the test_mle_diag_v3 branch May 8, 2024 15:00

This was referenced May 16, 2024

GFDL to main (2024-05-16) mom-ocean/MOM6#1626

Closed

GFDL to main 2024-05-16 mom-ocean/MOM6#1627

Closed

marshallward mentioned this pull request May 31, 2024

GFDL to main (2024-05-31) mom-ocean/MOM6#1631

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update MOM_mixed_layer_restrat.F90 #568

Update MOM_mixed_layer_restrat.F90 #568

marshallward commented Feb 8, 2024

marshallward commented Feb 8, 2024

codecov bot commented Feb 8, 2024 •

edited

Loading

Hallberg-NOAA left a comment

marshallward commented Feb 9, 2024

marshallward commented Feb 9, 2024 •

edited

Loading

amoebaliz commented Feb 9, 2024 via email

marshallward commented Feb 9, 2024 •

edited

Loading

marshallward commented Feb 9, 2024

marshallward commented Feb 13, 2024

Hallberg-NOAA Feb 17, 2024

marshallward Feb 20, 2024

Hallberg-NOAA left a comment

marshallward commented Feb 20, 2024

Hallberg-NOAA Feb 25, 2024

marshallward Feb 25, 2024

Hallberg-NOAA Feb 25, 2024

Hallberg-NOAA left a comment

Hallberg-NOAA Feb 25, 2024

Hallberg-NOAA commented Feb 25, 2024

Update MOM_mixed_layer_restrat.F90 #568

Update MOM_mixed_layer_restrat.F90 #568

Conversation

marshallward commented Feb 8, 2024

marshallward commented Feb 8, 2024

codecov bot commented Feb 8, 2024 • edited Loading

Codecov Report

Hallberg-NOAA left a comment

Choose a reason for hiding this comment

marshallward commented Feb 9, 2024

marshallward commented Feb 9, 2024 • edited Loading

amoebaliz commented Feb 9, 2024 via email

marshallward commented Feb 9, 2024 • edited Loading

marshallward commented Feb 9, 2024

marshallward commented Feb 13, 2024

Hallberg-NOAA Feb 17, 2024

Choose a reason for hiding this comment

marshallward Feb 20, 2024

Choose a reason for hiding this comment

Hallberg-NOAA left a comment

Choose a reason for hiding this comment

marshallward commented Feb 20, 2024

Hallberg-NOAA Feb 25, 2024

Choose a reason for hiding this comment

marshallward Feb 25, 2024

Choose a reason for hiding this comment

Hallberg-NOAA Feb 25, 2024

Choose a reason for hiding this comment

Hallberg-NOAA left a comment

Choose a reason for hiding this comment

Hallberg-NOAA Feb 25, 2024

Choose a reason for hiding this comment

Hallberg-NOAA commented Feb 25, 2024

codecov bot commented Feb 8, 2024 •

edited

Loading

marshallward commented Feb 9, 2024 •

edited

Loading

marshallward commented Feb 9, 2024 •

edited

Loading