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

+Add and use post_product_[uv] #22

Merged
merged 3 commits into from
Dec 7, 2021

Conversation

Hallberg-NOAA
Copy link
Member

This PR adds four new routines to within MOM_diag_manager.F90 to write out a
diagnostic based on the product of two arrays at velocity points, either as a
3-d diagnostic or a 2-diagnostic of the vertical sum. The second commit in this
PR uses this new set of interfaces to simplify and clarify the diagnostic code
in 4 modules. All answers are bitwise identical, but there are new public
interfaces. The commits in this PR include:

  • 2d053e6cb Use post_product_u for momentum budget diagnostics
  • 3aad8ca44 +Add post_product_[uv] and post_product_sum_[uv]

  Added four new routines to within MOM_diag_manager.F90 to write out a
diagnostic based on the product of two arrays at velocity points, either as a
3-d diagnostic or a 2-diagnostic of the vertical sum.  Doing it this way both
simplifies the higher level code and promotes the reuse of scratch arrays for
diagnostics.  In tests that use these new interfaces, diagnostics are identical
to their previous values.  All answers are bitwise identical, but there are 4
new public interfaces.
  Use the new post_product_[uv] and post_product_sum_[uv] routines to simplify
the code calculating derived diagnostics of the momentum budget in
step_MOM_dyn_split, calculate_diagnostic_fields(), horizontal_viscosity(),
vertvisc(), and layered_diabatic().  Also relocated the units for several
diagnostics to the same lines as their conversion factors in some calls to
register_diag_field, to make it easier to spot diagnostics with inconsistent
reported units, and removed variables that are no longer used.  All answers and
diagnostics are bitwise identical.
@codecov
Copy link

codecov bot commented Dec 3, 2021

Codecov Report

Merging #22 (ef795df) into dev/gfdl (7e1474e) will decrease coverage by 0.09%.
The diff coverage is 59.59%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl      #22      +/-   ##
============================================
- Coverage     29.17%   29.08%   -0.10%     
============================================
  Files           240      240              
  Lines         71491    71298     -193     
============================================
- Hits          20856    20735     -121     
+ Misses        50635    50563      -72     
Impacted Files Coverage Δ
src/parameterizations/lateral/MOM_hor_visc.F90 43.63% <33.33%> (-0.60%) ⬇️
src/core/MOM_dynamics_split_RK2.F90 61.90% <41.17%> (+0.88%) ⬆️
...parameterizations/vertical/MOM_diabatic_driver.F90 39.26% <50.00%> (-0.04%) ⬇️
src/diagnostics/MOM_diagnostics.F90 75.28% <100.00%> (-0.37%) ⬇️
src/framework/MOM_diag_mediator.F90 54.40% <100.00%> (+0.30%) ⬆️
...c/parameterizations/vertical/MOM_vert_friction.F90 42.47% <100.00%> (-1.78%) ⬇️
... and 1 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 7e1474e...ef795df. Read the comment docs.

@marshallward
Copy link
Member

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

Copy link
Member

@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.

I agree with these changes, especially moving the scratch arrays onto stack (in this case, by moving them into function calls).

It's not clear to me how this would extend to more complex operations, but we can address that in the future.

@marshallward marshallward merged commit c0be5d3 into NOAA-GFDL:dev/gfdl Dec 7, 2021
@Hallberg-NOAA Hallberg-NOAA deleted the post_product branch January 3, 2022 13:37
@Hallberg-NOAA Hallberg-NOAA added the refactor Code cleanup with no changes in functionality or results label Feb 1, 2022
marshallward pushed a commit that referenced this pull request Jul 2, 2022
* initial hooks for stochastic EOS modifications

* remove debug statements

* add documentation

* Change ampltiude from 0.39 to sqrt(.39)

* remove global_indexing logic from stoch_eos_init

* switch to using MOM_random and add restart capability

* update random sequence to update each each time-step

* remove tseed0 from MOM_random (leftover from debugging)

* Added necessary submodules and S^2, T^2 diagnostics to MOM_diagnostics

* Added diagnostics for outputting variables related to the stochastic parameterization.

* Diagnostics in MOM_PressureForce_FV updated for stochastic (rather than deterministic) Stanley SGS T variance parameterization.

* Added parentheses for reproducibility.

* Changed diagnostics to account for possible absence of stoch_eos_pattern in MOM_PressureForce_FV,
when deterministic parameterization is on.

* remove mom6_da_hooks and geoKdTree from pkg

* add stochastic compoment to MOM_thickness_diffuse

* fix array size declaration and post_data

* Corrected indexing of loops in MOM_calc_varT

* Changed how parameterization of SGS T variance (deterministic and stochastic) is switched on in PGF and thickness diffusion codes

* Corrected a few typos

* Cleaned up indices, redundant diagnostic, printing

* Fixed diagnostic IDs

* Fixed diagnostics typo

* Corrected indices in calculation of tv%varT

* Minor index fix

* Corrected bug in pressure in Stanley diagnostics

* Fixed whitespace error

* Stoch eos clock (#5)

*Added a clock for the Stanley parameterization

Co-authored-by: jkenigson <jkenigso@gmail.com>

* add halo update to random pattern

* Update MOM_stoch_eos.F90

Fix bug for looping over compute domain (is -> isc etc.)

* Avoid unnessary computations on halo (MOM_stoch_eos) and code clean-up (MOM_thickness_diffuse)

* Removed halo updates before determ param calc

* Update MOM_stoch_eos.F90

Removed unnecessary code

* Bug - indices are transposed

* Changed Stanley stochastic coefficient from exp(X) to exp(aX) (#9)

* Changed Stanley stochastic coefficient from exp(X) to exp(aX)

* Extra spaces removed

* Stoch eos init fix (#10)

* Don't bother calculating tv%varT if stanley_coeff<0

* Missing then added

* Merge Ian Grooms Tvar Discretization (#11)

* Update MOM_stoch_eos.F90

In progress updating stencil for$ | dx \times \nabla T|^2$ calculation

* New discretization of |dx\circ\nablaT|^2

Co-authored-by: Ian Grooms <ian.grooms@colorado.edu>

* Multiplied tvar%SGS by grid cell thickness ratio

* Added limiter for tv%varT

* Stoch eos ncar linear disc (#12)

* Update MOM_stoch_eos.F90

In progress updating stencil for$ | dx \times \nabla T|^2$ calculation

* New discretization of |dx\circ\nablaT|^2

* AR1 timescale land mask

Adds land mask to the computation of the AR1 decorrelation time

* Update dt in call to MOM_stoch_eos_run

The call to `MOM_stoch_eos_run` (which time steps the noise) is from within `step_MOM_dynamics`. `step_MOM_dynamics` advances on time step `dt` (per line 957), but the noise is updated using `dt_thermo`. It seems more appropriate to update the noise using `dt`, since it gets called from within `step_MOM_dynamics`.

* Fixed the units for r_sm_H

* Remove vestigial declarations

The variables `hl`, `Tl`, `mn_T`, `mn_T2`, and `r_sm_H` are no longer used, so I removed their declarations and an OMP private clause

Co-authored-by: Ian Grooms <ian.grooms@colorado.edu>

* Update MOM_thickness_diffuse.F90

Changed index for soft convention

* Update CVMix-src

* Ensure use_varT, etc., initialized

* Don't register stanley diagnostics if scheme is off

* Stanley density second derivs at h pts (#15)

* Change discretization of Stanley correction (drho_dT_dT at h points)

* Limit Stanley noise, shrink limiting value

* Revert t variance discretization

* Reverted variable declarations

* Stanley scheme in mixed_layer_restrat, vert_fill in stoch_eos, code cleanup (#19)

* Test Stanley EOS param in mixed_layer_restrat

* Fix size of TS cov, S var in Stanley calculate_density calls

* Test move stanley scheme initialization

* Added missing openMP directives

* Revert Stanley tvar discretization (#18)

* Perform vertical filling in calculation of T variance

* Variable declaration syntax error, remove scaling from get_param

* Fix call to vert_fill_TS

* Code cleanup, whitespace cleanup

Co-authored-by: Jessica Kenigson <jessicak@cheyenne1.cheyenne.ucar.edu>

* Use Stanley (2020) variance; scheme off at coast

* Comment clean-up

* Remove factor of 0.5 in Tvar

* Don't calculate Stanley diagnostics on halo

* Change start indices in stanley_density_1d

* Stanley param in MOM_isopycnal_slopes (#22)

Stanley param in MOM_isopycnal_slopes and thickness diffuse index fix

* Set eady flag to true if use_stored_slopes is true

* Cleanup, docs, whitespace

* Docs and whitespace

* Docs and whitespace

* Docs and whitespace

* Whitespace cleanup

* Whitespace cleanup

* Clean up whitespace

* Docs cleanup

* use_stanley

* Update MOM_lateral_mixing_coeffs.F90

* Adds link to another TEOS10 module

* Set Stanley off for testing

* Line continuation

Co-authored-by: Phil Pegion <38869668+pjpegion@users.noreply.github.com>
Co-authored-by: Philip Pegion <Philip.Pegion@noaa.gov>
Co-authored-by: Jessica Kenigson <jessicak@cheyenne6.cheyenne.ucar.edu>
Co-authored-by: Jessica Kenigson <jessicak@cheyenne3.cheyenne.ucar.edu>
Co-authored-by: jkenigson <jkenigso@gmail.com>
Co-authored-by: jskenigson <jessica.kenigson@colorado.edu>
Co-authored-by: Jessica Kenigson <jessicak@cheyenne1.cheyenne.ucar.edu>
Co-authored-by: Jessica Kenigson <jessicak@cheyenne5.cheyenne.ucar.edu>
Co-authored-by: Philip Pegion <ppegion@Philips-MacBook-Pro.local>
Co-authored-by: Jessica Kenigson <jessicak@cheyenne4.cheyenne.ucar.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code cleanup with no changes in functionality or results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants