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

Dev master candidate NCAR 03/27/2020 #1081

Conversation

gustavo-marques
Copy link
Collaborator

@gustavo-marques gustavo-marques commented Mar 28, 2020

Summary:

  • Add module MOM_lateral_boundary_diffusion. This module calculates and applies diffusive fluxes as a parameterization of lateral mixing (non-neutral) by mesoscale eddies near the top and bottom (to be implemented) boundary layers of the ocean. See #135 for further details

  • Create NCAR fork of Doxygen #123

  • Modify MEKE to allow GME to work properly #128

  • Adds option to scale KHTH with depth #127

  • Add option to pass river runoff via data_override in the NUOPC cap #130

  • Create a separate param for MEKE bottom drag #131

  • Fix bugs in Leith add new input parameter #134

  • Fix bugs caught by GNU compiler #141

  • Fix a bug in thickness_diffuse when using MEKE_GEOMETRIC #143

  • Update GME by removing its dependency on MEKE #144

  • OMP directives fixes #146, #142

This PR does not change answers for the GFDL tests when evaluated against the latest dev/master (intel/17.0.1). However, it adds the following runtime parameters:

MEKE_GEOMETRIC_ALPHA = 0.05     !   [nondim] default = 0.05
                                ! The nondimensional coefficient governing the efficiency of the GEOMETRIC
                                ! thickness diffusion.
MEKE_EQUILIBRIUM_RESTORING = False !   [Boolean] default = False
                                ! If true, restore MEKE back to its equilibrium value, which is calculated
                                ! ateach time step.
CDRAG_MEKE = 0.003              !   [nondim] default = 0.003
                                ! CDRAG is the drag coefficient relating the magnitude of the velocity field to
                                ! the bottom stress.
DEPTH_SCALED_KHTH = False       !   [Boolean] default = False
                                ! If true, KHTH is scaled away when the depth is shallower than a reference
                                ! depth: KHTH = MIN(1,H/H0)**N * KHTH, where H0 is a reference depth, controlled
                                ! via DEPTH_SCALED_KHTH_H0, and the exponent (N) is controlled via
                                ! DEPTH_SCALED_KHTH_EXP.
ADD_LES_VISCOSITY = False       !   [Boolean] default = False
                                ! If true, adds the viscosity from Smagorinsky and Leith to the background
                                ! viscosity instead of taking the maximum.
NDIFF_INTERIOR_ONLY = False     !   [Boolean] default = False
                                ! If true, only applies neutral diffusion in the ocean interior.That is, the
                                ! algorithm will exclude the surface and bottom boundary layers.
USE_LATERAL_BOUNDARY_DIFFUSION = False !   [Boolean] default = False
                                ! If true, enables the lateral boundary tracer's diffusion module.

ashao and others added 30 commits September 4, 2019 16:42
First stab at parameterizing the diabatic mixing by mesoscale eddies
using a 'bulk layer' approach. Added a simple unit test where column
thickness is exactly equal to the boundary layer depth, equal,
layer thicknesses, and the tracer gradient points from right to left.

Go Gustavo and Andrew
Add more complex unit tests and begin work on improving the algorithm
to deal with cases where the boundary layer intersects within a layer.
For the near-boundary lateral mixing, the indices of the layers that
are spanned by the boundary layer need to be returned. Additionally,
in cases where the boundary layer intersects partway through a layer,
the non-dimensional position also needs to be returned for polynomial
reconstructions to be evaluated correctly. Six unit tests were added
to test this new function.

All unit tests currently pass
Many updates to allow the boundary layer to intersect a layer.
Commented out some of the unit test previously added as the
API has changed. These need to be revisited later.
- Add two unit tests for cases where the surface boundary layer
intersects partly through a cell.
  1. Right column same BLT, same thicknesses, flux from right to left,
  constant in the vertical
  2. Right column same BLT, same thicknesses, flux from right to left,
  linear profile on right
TODO:
  1. Uncomment out previous unit tests
  2. Update API in those test cases
  3. Need to add similar unit tests for the bottom boundary
This updates all the previously commented out unit tests to update the
API. These changes were required to allow for cases where the boundary
layer that intersects partway through a model layer.
All the development in the boundary layer mixing scheme has focused on
simple unit tests. This provides a skeleton for some of the interfaces
that will need to be in place before using the new parameterization in a
'real' MOM6 simulation
- Pass diabatic CS through tracer_hor_diff_init and
lateral_boundary_mixing_init.
- Modify extract_diabatic_member to return KPP and ePBL CS
- Finish initialization for lateral_boundary_mixing
The new lateral boundary mixing routine has been added into
tracer_hor_diff and needs to be tested in a 'real' configuration.
This only works with KPP for now because ePBL needs US passed which is
not currently implemented in the API for tracer_hor_diff
Calculation of fluxes needs to be masked otherwise NaNs will
definitely be calcualted
The CVMix KPP module would allocate it's control structure regardless
of wthether KPP was used or not. The allocate statement has been moved
down after USE_KPP has been parsed.
- Indexing error in the y-direction led to a non-conservation of
  tracer
- Extra guards added to avoid divisions by zero
- Pass US through to lateral_boundary_mixing to enable compatibility
  with ePBL
Diffusive fluxes calculated from the lateral boundary mixing scheme
of tracers have been added as a diagnostic to the tracer registry.
The total 'bulk' flux was added as well
The get_MLD and get_BLD routines only return boundary layer depths
on the T-grid's computational domain leading to striping when
calculating the LBM fluxes. Adding a halo update for this variable
fixes the problem
TODO:
* add code for boundary = BOTTOM
* add unit tests
alperaltuntas and others added 15 commits March 6, 2020 16:27
Fix OMP race conditions and some chkcsum calls
Following changes have been made:

* add new argument (thickness_diffuse_CS) to horizontal_viscosity;
this is needed so that the GM coeff can be called from MOM_hor_visc.

* Deletes unnecessary calls to pass_vector

* Simplifies GME by removing dependecy on MEKE. GME is now set to be
some multiple of the GM coeff. A new runtime parameter (GME_efficiency)
can be used to control the strength of GME.
…de_MEKE

Updates GME by removing dependency on MEKE
Fix thread-unsafe computations of advective tracer flux vertical sums
@codecov-io
Copy link

codecov-io commented Mar 28, 2020

Codecov Report

Merging #1081 into dev/master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           dev/master    #1081   +/-   ##
===========================================
  Coverage       45.30%   45.30%           
===========================================
  Files             213      213           
  Lines           63234    63234           
===========================================
  Hits            28648    28648           
  Misses          34586    34586           

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 0cddf1f...0cddf1f. Read the comment docs.

@adcroft
Copy link
Collaborator

adcroft commented Mar 30, 2020

As per usual can @awallcraft, @kshedstrom and @jiandewang evaluate this PR.

Copy link
Collaborator

@adcroft adcroft left a comment

Choose a reason for hiding this comment

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

I've run this through our tests which all passed (as you knew it would).

I especially liked the bug catch (NCAR#141) that is a good example of why we perhaps should not allow single-line multi-statements. Thank goodness for the gnu compiler!

My only query is about the naming of CDRAG_MEKE. With one exception, every other MEKE parameter is named MEKE_* so could we have this renamed to MEKE_CDRAG for consistency?

@marshallward
Copy link
Collaborator

I've run the (not-yet-merged) rotational tests and these tests have passed.

And I second the proposal to reconsider phasing out multi-statement lines!

@gustavo-marques
Copy link
Collaborator Author

I've run this through our tests which all passed (as you knew it would).

I especially liked the bug catch (NCAR#141) that is a good example of why we perhaps should not allow single-line multi-statements. Thank goodness for the gnu compiler!

My only query is about the naming of CDRAG_MEKE. With one exception, every other MEKE parameter is named MEKE_* so could we have this renamed to MEKE_CDRAG for consistency?

Done.

@adcroft
Copy link
Collaborator

adcroft commented Mar 30, 2020

With the new parameter name GFDL team is ready to accept this PR. Now over to @jiandewang, @awallcraft @kshedstrom

@kshedstrom
Copy link
Collaborator

I approve this PR.

1 similar comment
@awallcraft
Copy link
Collaborator

I approve this PR.

@jiandewang
Copy link
Collaborator

jiandewang commented Apr 1, 2020

works fine in NCEP UFS, approve this PR

@adcroft adcroft self-assigned this Apr 1, 2020
@adcroft adcroft merged commit e73db38 into mom-ocean:dev/master Apr 1, 2020
@gustavo-marques gustavo-marques deleted the dev-master-candidate-ncar-2020-03-27 branch May 5, 2020 15:00
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.