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

NCAR sponge merge #1308

Merged
merged 28 commits into from
Feb 4, 2021
Merged

Conversation

MJHarrison-GFDL
Copy link
Contributor

@MJHarrison-GFDL MJHarrison-GFDL commented Jan 28, 2021

This PR contains updates for u,v accelerations in MOM_ALE_sponge provided by NCAR.

  • A bug was identified in the case of incoming sponge data residing on the model horizontal grid
    where the returned mask was not initialized properly.
  • The 2018 answers flags are being used to retain the bug and should be set to False for new experiments.
  • Enable UV sponges by setting SPONGE_UV=True
  • Sponge accelerations use the tracer timescale by default but can be set independently.
  • New sponge tendency diagnostics added (sp_tendency_{temp,salt} and sp_tendency_{u,v})

alperaltuntas and others added 25 commits July 1, 2020 18:48
  - This commit causes tc4 to fail.
  - sp_tendency_temp and sp_tendency_salt are new diagnostic
    variables which evaluate the respective tracer tendendies
    (tr_units/sec).
  - sp_tendency_u and sp_tendency_v diagnose the accelerations
    (m/sec) applied from calls to the sponge routine.
  - No attempt has been made to CMOR-ize these diagnostics.
  - More work is needed to generalize this code for additional tracers.
  - this PR includes updates from NCAR for uv momentum
    sponge implementation.
  - Diagnostics are included for tracer tendencies and
    accelerations due to sponge terms.
  - The uv sponge feature is currently not being tested. This will be
    addressed in a future PR which will add sponge accelerations
    to tc4.
  - A previous bug in the 3-dimensional mask returned from this
    routine in the case where the data to be interpolated reside
    on the model's horizontal grid is retained using the 2018_answers flag.
  - If 2018_answers is set to True, the mask is not properly initialized
    and this leads to incorrect vertical reconstructions in the ALE
    sponge code.
  - 2018 flags (DEFAULT_2018_ANSWERS or HOR_REGRID_2018_ANSWERS)
    should be set to False for any cases using sponge restoring for
    tracers or momentum.
@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #1308 (7a5697c) into dev/gfdl (1ae4e16) will decrease coverage by 0.04%.
The diff coverage is 30.04%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl    #1308      +/-   ##
============================================
- Coverage     45.94%   45.90%   -0.05%     
============================================
  Files           234      234              
  Lines         72121    72269     +148     
============================================
+ Hits          33138    33177      +39     
- Misses        38983    39092     +109     
Impacted Files Coverage Δ
src/parameterizations/vertical/MOM_ALE_sponge.F90 26.61% <25.43%> (+0.09%) ⬆️
src/initialization/MOM_state_initialization.F90 29.97% <31.03%> (-0.38%) ⬇️
src/core/MOM.F90 65.63% <75.00%> (ø)
src/framework/MOM_horizontal_regridding.F90 60.19% <100.00%> (+0.15%) ⬆️

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 1ae4e16...b950b05. 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.

I have examined these changes and they seem well thought-out and useful. The issues that were raised with some of the versions have been addressed, and this PR has passed both the TC testing and the pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/12000. I think that it is ready to be merged into MOM6.

However, because of the large number of commits (28) within this PR relative to the changes, and the fact that a number of the intermediate commits had problems that would preclude their use in any kind of Git bisection, I am going to do this as a squash-merge.

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.

4 participants