-
Notifications
You must be signed in to change notification settings - Fork 232
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
Main candidate from NCAR (12/15/2020) #1275
Main candidate from NCAR (12/15/2020) #1275
Conversation
* turned off pressure force * hard-coded BLD * turned off advect_tracer * set NTR = 2
This commit adds a linear transition from full LBD at k=k_min to zero LBD at k=k_max. This is applied to both methods currently available in the LBD module. Another modification is the fact that both methods no longer compute average values at k_min (done previously via average_value_ppoly). Instead, the full layer thicknesses are now used.
This patch adds the option to apply a linear decay of the fluxes at the base of hbl. This had been already implemented but since it breaks the unit tests, which were designed to work without this option, adding this option will avoid breaking the tests.
When using the option to apply neutral diffusion only below the surface boundary layer we were using (1.-zeta). This is wrong. It should be just (zeta).
Valid options are: * PARAM - use the vector-parameter LBD_DZ_TOP * UNIFORM[:N] - uniformly distributed * FILE:string - read from a file
For each tracer point: * tracers at (I,j), (I+1,j), (i,J) and (i,J+1) are remapped to a defined zgrid; * Apply LBD, uflux and vflux are calculated using the zgrid * Remap fluxes to native grid * Apply tracer convergence in the native grid TODO: * cleanup * create a zgrid for each pair of column taking into consideration h_L, h_R, BLD_L and BLD_R.
* Add functions to merge thicknesses and BLDs * z_top is now defined every time-step using this information * added unit tests
* adding new functions to sort, swap, and remove duplications in 1D arrays * updating unit tests * clean the module
…idate-ncar-2020-08-11
A few fixes to get it to compile with gfortran.
Introduce a controllable stdout unit in MOM_io
…ed_10nov2020 Major updates to the LBD module
@gustavo-marques I am seeing a
Do you want to look into this? Can you try to reproduce? |
@jiandewang, @kshedstrom, @abozec (or @awallcraft) can you review/test this PR per usual process? We're fairly happy with it other than the above unit test issue but continue to test. |
@marshallward, it passed all unit tests with Intel 19.0.5. Let me try with GNU... |
I get a fail on Intel 18.0.6 (with slightly more info since it's a REPRO build):
I'll try to repeat this with Intel 19. |
Okay, I found the problem (I thought I had merged this NCAR#169, but apparently I did not). I will send a fix in a few. |
I approve this PR. |
I still seem to be getting a fail, although I am now getting different answers (-0.4 rather than 0.0):
Could be something to do with the merge; I will pick this up tomorrow. |
I just tried an Intel "REPRO" build and it did pass, so there could still be an underlying problem here (e.g. assumed zero initialization?). Edit: PGI also passes, so this problem seems restricted to GNU. I don't know anything about this test, so I won't be able to sort this out quickly, but I'll start looking into it. In the meantime, can you re-try your test using GNU? |
Thanks, @marshallward. I will try to reproduce the fail with GNU. |
I approve this PR. |
@marshallward, I was able to reproduce the fail with GNU in debug mode. The test is failing because of an undefined logical (CS%limiter_remap) in the unit test function (near_boundary_unit_tests). For some reason, GNU/debug sets it to True by default. The fix coming in a few. |
EMC wcoss machine is reserved for production test only at this moment, will test this MOM code once access to machine is back |
Thanks, @gustavo-marques , good to see it was nothing serious. This is now passing on our branch with no regressions (after resolving a minor git conflict). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have visually examined all of these code changes, and I approve of them. (I especially like the use of the deferred allocation character strings, which is a very elegant solution!) Also, I have been informed that this PR is now passing all of the testing from the GFDL side.
The merge of this PR onto the main branch is now just waiting for the go-ahead from the EMC team via @jiandewang, having obtained approval from the other main forks. |
I shall have access to machine this afternoon, will try to finish testing no later than today |
works fine on EMC (PS: I started my testing 1 minute after machine is back, so my job got finished quickly as nodes are empty) |
Approvals:
I am going to merge this now. |
Major changes
Make long character variables deferred length (Make long character variables deferred length NCAR/MOM6#165)
Major updates to the lateral boundary diffusion module (Major updates to the LBD module NCAR/MOM6#164)
Introduce a controllable stdout unit in MOM_io (Introduce a controllable stdout unit in MOM_io NCAR/MOM6#163)
New diagnostics
This candidate does not changes answers for the MOM6 regression tests (Intel 19.0.5).