Momentum budget terms multiplied by fractional layer-thicknesses #1163

hmkhatri · 2020-07-18T19:35:48Z

New diagnostics, i.e. momentum budget terms (PFu/PFv, CAu/CAv etc.) multiplied by fractional layer thicknesses have been added for momentum budget analysis. Fractional thicknesses are already computed in the barotropic step. These fractional thicknesses are now returned in the btstep subroutine call in the MOM_dynamics_split_RK2 file and are used across different modules to compute required diagnostics.

These depth-integrated budget term diagnostics would be useful in studying barotropic momentum and vorticity budgets, and the depth-integrated budget terms are also implemented in the diagnostics.

@ashao @StephenGriffies @Hallberg-NOAA Could you please review it?

…ed. In these, differet budget terms multiplied by fractional layer thicknesses are saved. Thus, these terms can be added over the whole to obtain the barotropic momentum budget. 'btstep' subroutine in MOM_barotropic module is modified to return fractional thicknesses. Pressure force accelaration multiplied by fractional thickness as diagnostics are added in this commit.

…lied momentum budget terms

…ative vorticity and gradient of kinetic energy

…weighted)

…y allocated if any of the relevant diagnostics are called.

codecov-commenter · 2020-07-18T19:43:26Z

Codecov Report

Merging #1163 into dev/gfdl will decrease coverage by 0.13%.
The diff coverage is 42.11%.

@@             Coverage Diff              @@
##           dev/gfdl    #1163      +/-   ##
============================================
- Coverage     46.08%   45.95%   -0.14%     
============================================
  Files           214      224      +10     
  Lines         69399    70428    +1029     
============================================
+ Hits          31984    32363     +379     
- Misses        37415    38065     +650

Impacted Files	Coverage Δ
...g_src/external/GFDL_ocean_BGC/FMS_coupler_util.F90	`0.00% <0.00%> (ø)`
...fig_src/external/GFDL_ocean_BGC/generic_tracer.F90	`0.00% <0.00%> (ø)`
...c/external/GFDL_ocean_BGC/generic_tracer_utils.F90	`0.00% <0.00%> (ø)`
config_src/external/ODA_hooks/kdtree.f90	`0.00% <0.00%> (ø)`
config_src/external/ODA_hooks/ocean_da_core.F90	`0.00% <0.00%> (ø)`
config_src/external/ODA_hooks/ocean_da_types.F90	`0.00% <0.00%> (ø)`
config_src/external/ODA_hooks/write_ocean_obs.F90	`0.00% <0.00%> (ø)`
config_src/solo_driver/MESO_surface_forcing.F90	`0.00% <0.00%> (ø)`
config_src/solo_driver/MOM_driver.F90	`68.72% <ø> (ø)`
config_src/solo_driver/user_surface_forcing.F90	`0.00% <0.00%> (ø)`
... and 150 more

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 df33724...b5b3030. Read the comment docs.

StephenGriffies · 2020-07-20T16:18:38Z

Thanks for this pull request @hmkhatri.

We spoke about it during the MOM6 developers meeting today, 20 July 2020. @Hallberg-NOAA and @adcroft are happy with the 2d diagnostics. However, they wish to comment out the 3d diagnostics given that we do not know the proper remapping grid option and to retain these diagnostics might confuse future users/developers.

Given that we might need to have these diagnostics for debugging in the future, we recommend commenting them and putting a note in the code at each comment. At some point in the near future, after multiple applications of these diagnostics over some months, then we can remove the commented code. But for the meantime, we should just leave the code but have it commented.

Please make these changes and update your pull request, at which point we will run through the regression tests and move forward with bringing this code into the main trunk.

Thanks again! This is a good step forward with making MOM6 an analyst-friendly code.

… we do not the proper grid remapping option.

hmkhatri · 2020-07-22T17:03:58Z

@StephenGriffies Thanks. I agree that the remapping option could be an issue in 3D diagnostics. As suggested, I have commented 3D diagnostics and put a note in all relevant places.

Hallberg-NOAA

This PR is getting close to where it needs to be, and it will be a valuable addition when it does get there.

However, there are still a few changes that are needed. First, this is still failing some of the the Travis tests, as indicated by the test report toward the bottom of https://github.com/NOAA-GFDL/MOM6/pull/1163. Looking into the details, there are a couple of new variables that are not being described in dOxygen comments, and some of the new line lengths exceed the 120 character MOM6 limit. Fixing these should not be difficult.

Travis is also reporting that the checksums in the files with all diagnostics have changed, but this is acceptable because this commit is introducing new diagnostics and these checksums are supposed to change.

In addition, there are some allocate statements for pointers (hf_rvxu_2d, hv_rvxv_2d, hf_gKEv_2d, and hf_gKEu_2d at least) that do not have corresponding deallocate statements, which could lead to a memory leak. Because these arrays are not used outside of CorAdCalc, they do not need to be pointers - they could be allocatable arrays. Also, since these are only 2-d, the memory-footprint consequences of making these into automatically allocated arrays (like KEx in the same file) would not be too bad. Please look at these and all the other pointers that are being allocated as a part of this commit to make sure that we are not introducing any memory leaks.

I look forward to seeing the updated version with these minor corrections that we can pull onto dev/gfdl. @hmkhatri, I can help guide you through these fixes if any of this is not clear.

StephenGriffies · 2020-07-23T21:36:04Z

Thanks @Hallberg-NOAA for your careful review. These changes should be straightforward.

hmkhatri · 2020-07-23T22:45:27Z

Thanks for the detailed comments @Hallberg-NOAA. I do not understand what you mean by "dOxygen comments".

I will change 2D arrays from pointer style to allocatable local arrays and deallocate them after use. Just to be sure, I should use the following style and deallocate after use, right?

real, allocatable, dimension(:,:) :: array_u
allocate(array_u(G%IsdB:G%IedB,G%jsd:G%jed))
if (allocated(array_u)) deallocate(array_u)

StephenGriffies · 2020-07-24T00:03:43Z

Take a look here for documentation of MOM6 code

https://github.com/NOAA-GFDL/MOM6/wiki

In particular, see this page for Doxygen

https://github.com/NOAA-GFDL/MOM6/wiki/Doxygen

…llocatable array style.

…acters

hmkhatri · 2020-07-27T16:29:01Z

@Hallberg-NOAA @StephenGriffies Thanks for the comments. I have implemented the suggested changes. All 3D diagnostics are now commented. For 2D diagnostics, allocatable arrays are used for initialization and these arrays are deallocated after the post-data step. All lines are under 120 characters in length.

adcroft · 2020-07-27T20:35:52Z

Testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/10886

Hallberg-NOAA

Thank you for the revisions to this PR. They have neatly addressed all of the previous issues, and I am now happy to approve this PR.

Hallberg-NOAA · 2020-07-28T14:22:19Z

This passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/10886 . It changes the contents of the MOM_parameter_doc files, so there will need to be an update to MOM6-examples after this PR is resolved on the MOM6 repository.

hmkhatri added 22 commits May 28, 2020 18:42

More thickness weighted diagnostics

3124b4d

Implemented Andrew Shao's suggestions

d9e1ec2

More fractional thickness multiplied diagnostics

2a0ee28

Some barotropic diagnostics obtained from fractional thickness multip…

a18cd08

…lied momentum budget terms

Removed trailing spaces

bd29c37

All fractional-thickness multiplied diagnostics implemented

192fb9b

define diagnostic variables as pointers

fb119b0

Merge branch 'dev/gfdl' into diag_TW

a7173e0

Shorter initlization of 2D arrays

6de102b

Modifications in vertical friction diagnostics

8f30470

Diagnotics initilization as pointers to save memory allocation

3a9a944

Vertical friction module

53b5c77

Removed commented lines and trailing spaces

ff54401

Diagnostic description change

d212cc6

Diagnostic description change

3d819c8

Fractional thickness-weighted diagnostics for accelaration due to rel…

7d2b052

…ative vorticity and gradient of kinetic energy

Modifications in vertical friction diagnostics (fractional thickness-…

4dc88c3

…weighted)

Merge branch 'dev/gfdl' into diag_TW

20fe0e5

Modifications in diag_hfrac_u and diag_hfrac_v calls

ba0aa2f

Made allocation of hfrac at u/v points optional. These arrays are onl…

c815434

…y allocated if any of the relevant diagnostics are called.

Merge branch 'dev/gfdl' into diag_TW

306e18b

hmkhatri changed the title ~~Momentum budget terms multiplied by factional layer thicknesses~~ Momentum budget terms multiplied by fractional layer-thicknesses Jul 18, 2020

Fractional-thickness weighted 3D diagnostics are now commented out as…

428e61f

… we do not the proper grid remapping option.

Hallberg-NOAA reviewed Jul 23, 2020

View reviewed changes

hmkhatri added 3 commits July 27, 2020 12:14

Initilization of 2D diagnostic arrays changed from pointer stype to a…

e2ede33

…llocatable array style.

Trailing spaces removed and line lengths reduced to be under 120 char…

082f3b5

…acters

Comment modified

8067c5b

Merge branch 'dev/gfdl' into diag_TW

b5b3030

Hallberg-NOAA approved these changes Jul 28, 2020

View reviewed changes

adcroft merged commit 4c030e6 into mom-ocean:dev/gfdl Jul 29, 2020

hmkhatri deleted the diag_TW branch September 15, 2020 18:06

This was referenced Sep 18, 2020

main-candidate-gfdl-2020-09-18 MOM6/MOM6#1

Merged

Candidate for main from GFDL 2020-09-18 #1211

Merged

hmkhatri mentioned this pull request Mar 1, 2021

Depth-integrated momentum budget diagnostics #1343

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Momentum budget terms multiplied by fractional layer-thicknesses #1163

Momentum budget terms multiplied by fractional layer-thicknesses #1163

hmkhatri commented Jul 18, 2020 •

edited

Loading

codecov-commenter commented Jul 18, 2020 •

edited

Loading

StephenGriffies commented Jul 20, 2020

hmkhatri commented Jul 22, 2020

Hallberg-NOAA left a comment

StephenGriffies commented Jul 23, 2020

hmkhatri commented Jul 23, 2020

StephenGriffies commented Jul 24, 2020

hmkhatri commented Jul 27, 2020

adcroft commented Jul 27, 2020

Hallberg-NOAA left a comment

Hallberg-NOAA commented Jul 28, 2020

Momentum budget terms multiplied by fractional layer-thicknesses #1163

Momentum budget terms multiplied by fractional layer-thicknesses #1163

Conversation

hmkhatri commented Jul 18, 2020 • edited Loading

codecov-commenter commented Jul 18, 2020 • edited Loading

Codecov Report

StephenGriffies commented Jul 20, 2020

hmkhatri commented Jul 22, 2020

Hallberg-NOAA left a comment

Choose a reason for hiding this comment

StephenGriffies commented Jul 23, 2020

hmkhatri commented Jul 23, 2020

StephenGriffies commented Jul 24, 2020

hmkhatri commented Jul 27, 2020

adcroft commented Jul 27, 2020

Hallberg-NOAA left a comment

Choose a reason for hiding this comment

Hallberg-NOAA commented Jul 28, 2020

hmkhatri commented Jul 18, 2020 •

edited

Loading

codecov-commenter commented Jul 18, 2020 •

edited

Loading