-
Notifications
You must be signed in to change notification settings - Fork 328
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
Convert GM-Redi to triad formulation #280
Convert GM-Redi to triad formulation #280
Conversation
b4808e4
to
878d24e
Compare
142cf57
to
023d761
Compare
Here is a quick overview. Redi mixing is a mesoscale eddy parameterization that mixes scalars across isopycnal surfaces. The formulation is: The current formulation in MPAS-Ocean was written 5 years ago, has never been used. The original design document is here: https://github.com/MPAS-Dev/MPAS-Documents/tree/master/ocean/gm. The current formulation produces spurious oscillations. See the test tracers on the bottom right: This noise problem was described in detail by Beckers et al. 1998 who say:
and
There is some help, though. Griffies et al 1998 introduces a "triad" expansion of the slope calculation, essentially his (30-34) that is total variance diminishing, and much better behaved for a nonlinear equation of state. It was used in MOM and POP. Though noise can still exist, it is much reduced. Specifically, the cross-terms produce over/undershoots beside horizontal boundaries, and the triad formulation does not help that. In practice, the vertical mixing smooths most of this out, but any Redi formulation is not locally bounds preserving. |
Here is an explanation of the triads. The horizontal flux is computed over four triads: Looking at a single triad, the density is expanded in temperature and salinity, and spatial derivatives are taken at each side edge and top boundary: Griffies et al 1998 defines the slope of the triad here: |
023d761
to
bb4ecf3
Compare
7a3a0be
to
7a196ff
Compare
@vanroekel or anyone else, the flags are a bit jumbled for GM and Redi. I've updated to move Redi into a separate list, since it is really a separate term on the tracer equations: CURRENT
REVISED, FOR THIS PR
where the new flag Let me know if anyone has comments on any flag names on this list. They are easy to change now. |
Currently testing with analytic functions in a periodic Cartesian domain. See symbolic algebra in python of test functions and solutions for each Redi term here: |
f73e169
to
641a588
Compare
728c4dc
to
529fba9
Compare
@maltrud @milenaveneziani @vanroekel I'm putting these flags in now. Let me know if you have any further suggestions.
|
9bbab59
to
9b5ab28
Compare
If we don't think we will ever have the kappas, res_min, and res_max different between GM and Redi, wouldn't it be clearer to have one section called 'eddy_parameterization' and then have the GM_Redi common config options, like we talked about in the hallway? |
I was on the fence. I like your idea of a separate namelist section that applies to all of them better. I'll post in a sec... |
Yes, this is better. Thanks for the suggestion:
|
9b5ab28
to
7c8d5d4
Compare
@mark-petersen the main problem i have with this is "all_eddy_parameterizations". when we add in, say, a submesoscale eddy parameterization this will be confusing. i suggest "GM_and_Redi_parameterizations" or something like that. |
I like @maltrud's idea. I would suggest "mesoscale_eddy_parameterizations" with the hope we one day move beyond GM/Redi. |
OK, currently at
Note altered name |
i think i'd prefer config_mesoscale_eddy_isopycnal_slope_limit |
Agreed, thanks. |
07a5933
to
e323790
Compare
@vanroekel, please approve, based on our conversation and the above testing. Also see overview summary at: |
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.
@mark-petersen I did a quick visual pass and most looks good. I think we should remove the new debug tracers and some of the temporary changes for debugging prior to merging though. I tried to comment where they are.
!$omp end do | ||
end if | ||
end if | ||
|
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.
@mark-petersen do we really want to merge these debug tracer changes into ocean/develop?
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 would like to merge in the reset of debug tracers now, because we will be conducting tests in the next few weeks where this makes it very easy to control with flags. After we are satisfied, we can remove this section when we turn on Redi mixing by default, with the tuned coefficients. Also, with this merge the debug reset will be on the trunk once, so it is easy to find later.
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.
Good point. I agree this would be good for testing.
@vanroekel Please approve when you are ready, and I will merge and update the E3SM PR. |
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.
approved based on visual inspection and @mark-petersen's extensive testing.
!$omp end do | ||
end if | ||
end if | ||
|
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.
Good point. I agree this would be good for testing.
Merged locally, tested with nightly regression suite one last time, both debug and optimized. |
Currently, the Redi along-isopycnal mixing formulation used simple spatial averaging to compute the isopycnal slope. This PR introduces triad averaging, which uses a very specific form of an expansion of the slope using three cells, the thermal expansion and saline contraction coefficients.
* ocean/develop: Add Gulf of Maine variable resolution mesh Revert "Merge PR #280 'ocean/redi_add_triads' into ocean/develop" Alter the e3sm_coupling README to add warnings runoff LI script: add argparse Separate functions for CORE, JRA, ne30 Add e3sm_coupling script. Add ARM60to10 and ARM60to6, with cartopy to plot coastlines Update AtlanticPacificGrid from matlab to python Add mirror for bathymetry_database Change from oceans11 to lcrc in compass/ocean Update the version of mpas_tools required for COMPASS First commit for modification of python scripts plotting four test cases
…ean/develop"" This reverts commit d3667cc.
Currently, the Redi along-isopycnal mixing formulation used simple spatial averaging to compute the isopycnal slope. This PR introduces triad averaging, which uses a very specific form of an expansion of the slope using three cells, the thermal expansion and saline contraction coefficients.
…ean/develop"" This reverts commit d3667cc.
…ean/develop"" This reverts commit d3667cc.
Currently, the Redi along-isopycnal mixing formulation used simple spatial averaging to compute the isopycnal slope. This PR introduces triad averaging, which uses a very specific form of an expansion of the slope using three cells, the thermal expansion and saline contraction coefficients.