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 from NCAR 2019/10/02 #1013

Conversation

gustavo-marques
Copy link
Collaborator

Summary:

  • Corrects time-related arguments in ocean_model_init in the MCT and NUOPC caps. This fixed the issue of overwriting ocean.stats every time the model was restarted;
  • Obsoletes ADD_KV_SLOW and removes visc%Kv_slow from restart files since it is no longer needed;
  • Checks for consistency of lat, lon and mask between mesh and mom6 grid in the NUOPC cap;
  • Fixes the coding issues with GME described in Coding issues with GME code NCAR/MOM6#109;
  • Adds NCAR version of MOM6 Doxygen (https://ncar.github.io/MOM6/APIs/), and improves NUOPC documentation.

Changes in answers and MOM parameters

The following parameter will be added in MOM_parameter_doc.all when USE_MEKE = True:

MEKE_EQUILIBRIUM_ALT = False    !   [Boolean] default = False
                                ! If true, use an alternative formula for computing the (equilibrium)initial
                                ! value of MEKE.

The following will be removed:

ADD_KV_SLOW = False             !   [Boolean] default = False
                                ! If true, the background vertical viscosity in the interior (i.e., tidal +
                                ! background + shear + convection) is added when computing the coupling
                                ! coefficient. The purpose of this flag is to be able to recover previous
                                ! answers and it will likely be removed in the future since this option should
                                ! always be true.

I could not figure out why the answers change for the following configuration (perhaps compiler version? I've used intel/17.0.1):

modified: regressions/coupled_AM2_LM3_SIS/CM2G63L/ocean.stats.intel

All other configurations in MOM6-examples are bit for bit.

gustavo-marques and others added 30 commits August 15, 2019 14:46
 Obsolete ADD_KV_SLOW and remove visc%Kv_slow from restart files
NOTE: we need to add a warning that MEKE_EQUILIBRIUM only works
if MEKE_KHCOEFF > 0.
Check for consistency of lat, lon and mask between mesh and mom6 grid
…ck_nuopc

Adds consistency check between mesh and mom6 grid
This will use an alternative formula for computing the equilibrium
(initial) value of MEKE.
Changed the loop indices on boundary mask. Added three new runtime parameters
for GME (GME_H0, GME_EFFICIENCY, GME_LIMITER)
@codecov-io
Copy link

codecov-io commented Oct 3, 2019

Codecov Report

❗ No coverage uploaded for pull request base (dev/master@5b994f3). Click here to learn what that means.
The diff coverage is 29%.

Impacted file tree graph

@@              Coverage Diff              @@
##             dev/master    #1013   +/-   ##
=============================================
  Coverage              ?   43.17%           
=============================================
  Files                 ?      213           
  Lines                 ?    62163           
  Branches              ?        0           
=============================================
  Hits                  ?    26836           
  Misses                ?    35327           
  Partials              ?        0
Impacted Files Coverage Δ
src/core/MOM_variables.F90 77.27% <ø> (ø)
src/diagnostics/MOM_obsolete_params.F90 87.66% <100%> (ø)
src/parameterizations/lateral/MOM_hor_visc.F90 62.14% <19.23%> (ø)
...c/parameterizations/vertical/MOM_set_viscosity.F90 65.22% <50%> (ø)
...c/parameterizations/vertical/MOM_vert_friction.F90 72.03% <50%> (ø)
...arameterizations/lateral/MOM_thickness_diffuse.F90 63.18% <61.53%> (ø)
src/parameterizations/lateral/MOM_MEKE.F90 69.52% <75%> (ø)

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 5b994f3...bccdf19. Read the comment docs.

Copy link
Collaborator

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

Thank you for these suggested changes. They are very helpful for cleaning up aspects of the code.

However, a number of the comments documenting the dimensional rescaling of variables in MOM_hor_visc.F90 have been removed or perhaps omitted by basing the changes on an older version of the code. Please consider restoring these comments (i.g., [T-1 ~> s -1] instead of [s-1] on line 244 of MOM_horvisc.F90, and similarly on numerous following lines.

@gustavo-marques
Copy link
Collaborator Author

@Hallberg-NOAA, sorry for missing the comments documenting the dimensional rescaling of variables. These have been restored now. Thank you for catching this.

@kshedstrom
Copy link
Collaborator

I approve this PR.

@marshallward
Copy link
Collaborator

marshallward commented Oct 4, 2019

The FrictWorkMax and FrictWork_diss diagnostics appear to have been removed, along with blocks of code dedicated to computing them. Was that intentional?

@marshallward
Copy link
Collaborator

Ignore my last comment, I see from ba2f186 that this was intentional. (This may require an update of the OpenMP directive, btw).

@marshallward
Copy link
Collaborator

Github is not reporting any conflicts here, but if I try to do a local merge then I get a conflict in src/parameterizations/lateral/MOM_hor_visc.F90 at L1379.

@gustavo-marques I'm not really sure the best way to resolve this on our end, but if you wanted to do a dev/gfdl merge into your PR, resolve the conflict, and then update the PR, maybe that would work?

@adcroft
Copy link
Collaborator

adcroft commented Nov 6, 2019

If there is no conflict with dev/master then all is good for this PR. Merging dev/master back into dev/gfdl is our problem. This PR was OK'd already (before we lost the machine) so complete the merge on dev/master and respectively update MOM6-examples and the stats repo. We'll look at merging onto dev/gfdl after that.

@marshallward
Copy link
Collaborator

Right, sorry, wrong branch. Will deal with this as a dev/master merge.

@marshallward marshallward merged commit ec31391 into mom-ocean:dev/master Nov 7, 2019
@gustavo-marques gustavo-marques deleted the dev-master-candidate-ncar-2019-10-02 branch September 23, 2020 16:38
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.

8 participants