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

Fixes in MEKE that allows GME to work properly #128

Merged
merged 8 commits into from
Oct 23, 2019

Conversation

gustavo-marques
Copy link
Collaborator

Summary:

  • Adds a new equilibrium formula for MEKE a4f9550;

  • Rearranges MEKE_EQUILIBRIUM subroutine to avoid unnecessary calculations 839217d;

  • Adds option to nudge MEKE to an equilibrium value. To select this option one needs to set MEKE_EQUILIBRIUM_RESTORING=True. The timescale for nudging is controlled via MEKE_RESTORING_TIMESCALE ebf5ee0.

This PR should not change answers since none of these options are enabled in our tests.

* Follow equation 1 of Jansen et al. (2015), balancing the GEOMETRIC GM coefficient against
bottom drag (Equations 3 and 12);
* Added limited for SN in this formula, to avoid extremely large values.

TODO:
* Increase GEOMETRIC_ALPHA in this calculation
* Use GEOMETRIC_EPSILON as a limiter for SN
In this commit, bottomFac2 is calculated when CS%MEKE_GEOMETRIC is
set to true. Previously, bottomFac2 was calculated in MEKE_lengthScales_0d
but something else on that subroutine was returning Nan so we decided to
pull out just the bottomFac2 calculation from that.
* Moved MEKE_equilibrium_alt toward top of subroutine to avoid unnecessary
calculations.
This commit adds a subroutine that calculates a new equilibrium value
for MEKE at each time step. This is not copied into MEKE%MEKE; rather,
it is used as a restoring term to nudge MEKE%MEKE back to an equilibrium value.
To select this option one needs to set MEKE_EQUILIBRIUM_RESTORING=True.
The timescale for nudging is controlled via MEKE_RESTORING_TIMESCALE.
This patch fixes the following MEKE diagnostics:

- MEKE_Ue, MEKE_Ub, MEKE_Ut

The diagnostics were computed as inline operations inside post_data,
e.g.:

    post_data(..., sqrt(0, max(0., MEKE*bottomFac2)))

rather than computing the fields explicitly inside of array loops.

This case causing floating point exceptions in Intel compilers, possibly
likely due to evaluations inside of halos.

We resolve these diagnostics by computing the values into a scratch
array which is then passed to post_data.
@gustavo-marques
Copy link
Collaborator Author

Test tc2 was failing because of some MEKE diagnostics. I've cherry-picked commit 3f041d9 from https://github.com/NOAA-GFDL/MOM6/pull/1014 that fixes these issues.

@marshallward
Copy link

I was able to reproduce this error on my desktop, and got the following error:

tc2.repro: At line 833 of file /home/marshall/gfdl/mom6_ncar/.testing//../src/parameterizations/lateral/MOM_MEKE.F90
tc2.repro: Fortran runtime error: Index '5' of dimension 2 of array 'cs%equilibrium_value' below lower bound of 2314885530818453536
tc2.repro: 
tc2.repro: Error termination. Backtrace:
tc2.repro: #0  0x55882b866c24 in ???
tc2.repro: #1  0x55882be1278d in ???
tc2.repro: #2  0x55882babd7d1 in ???
tc2.repro: #3  0x55882b7fdbbe in ???
tc2.repro: #4  0x7f27e3767152 in ???
tc2.repro: #5  0x55882b7fdbfd in ???
tc2.repro: #6  0xffffffffffffffff in ???
tc2.repro: --------------------------------------------------------------------------
tc2.repro: Primary job  terminated normally, but 1 process returned
tc2.repro: a non-zero exit code. Per user-direction, the job has been aborted.
tc2.repro: --------------------------------------------------------------------------
tc2.repro: --------------------------------------------------------------------------
tc2.repro: mpirun detected that one or more processes exited with non-zero status, thus causing
tc2.repro: the job to be terminated. The first process to do so was:
tc2.repro: 
tc2.repro:   Process name: [[16273,1],0]
tc2.repro:   Exit code:    2
tc2.repro: --------------------------------------------------------------------------
make: *** [Makefile:253: results/tc2/ocean.stats.repro] Error 1

I haven't looked at what's going on yet, but looks like CS%equilibrium_value may not have been initialized. Hard for me to say for sure, if it was set outside of the function.

One bit of good news is that the updated Makefile will report this information more clearly when it's merged into dev/master.

…ibrium_restoring

* Also deletes unneeded variables from subroutine MEKE_equilibrium_restoring.
@gustavo-marques
Copy link
Collaborator Author

gustavo-marques commented Oct 23, 2019

Thanks, @marshallward. I was able to reproduce the error on Cheyenne with gnu/8.3.0.
Allocating CS%equilibrium_value inside subroutine MEKE_equilibrium_restoring solves the problem.

@codecov-io
Copy link

Codecov Report

Merging #128 into dev/ncar will increase coverage by <.01%.
The diff coverage is 59.81%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/ncar     #128      +/-   ##
============================================
+ Coverage     43.16%   43.16%   +<.01%     
============================================
  Files           213      213              
  Lines         62198    62232      +34     
============================================
+ Hits          26849    26865      +16     
- Misses        35349    35367      +18
Impacted Files Coverage Δ
src/parameterizations/lateral/MOM_MEKE.F90 68.31% <59.81%> (-1.22%) ⬇️

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 2bb321e...050aa31. Read the comment docs.

@marshallward
Copy link

This is suggesting to me that we need to continue doing an optimized test run (REPRO=1), even if we don't necessarily expect agreement with the unoptimized results. I might modify the test to always run but only conditionally test the output.

@alperaltuntas alperaltuntas merged commit 462768b into NCAR:dev/ncar Oct 23, 2019
@gustavo-marques gustavo-marques deleted the fix_MEKE_GM_src 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.

4 participants