-
Notifications
You must be signed in to change notification settings - Fork 334
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
Update mpas-source for Langmuir ocean vertical mixing parameterization #2952
Conversation
Please see discussion on testing at MPAS-Dev/MPAS-Model#134. This Right now the 'Files changed' tab will include the changes in #2852. After that is merged, it will only show the changes for this PR. The mpas-source submodule currently points to the branch |
@jonbob and @vanroekel this branch is ready for the namelist updates and testing. |
01d94b0
to
1520c23
Compare
All the threading code is now merged. This PR is ready to go. @vanroekel and @qingli411, please look at the 'Files changed' above to make sure you see what you expect. Then run a few tests on this branch to make sure it is the same as your long testing previously. |
@mark-petersen Great. The 'Files changed' looks good to me. I will do some tests on this branch. |
Passed my standard tests:
Everything for the mpas-source is done. I won't be able to work further on this PR, but I think you all can finish it. @jonbob, there are three new namelist flags:
|
@jonbob should we append the buildnml changes to activate langmuir mixing to this PR? |
@jonbob @vanroekel These are the changes to buildnml for mpas-ocean I need to activate Langmuir mixing in E3SM: |
@qingli411 , @vanroekel - we have scripts that pick up changes in the Registry file and auto-generate something close to the right build-namelist files. So thanks for pointing me at the required changes, but that's not necessary. Are the defaults in Registry the ones you want in E3SM??
|
@jonbob I think we want in order
Is that right @qingli411? |
@vanroekel @jonbob If we want to have the Langmuir turbulence parameterization turned on by default, we need
There is another modification that we need. To use the 'theory wave' option we need 10-meter wind speed being passed from coupler to mpas-ocean in |
@qingli411 for second, you mean
right? I'm changing MPAS-Ocean to run stand-alone tests with these defaults as well. Mainly that just exercises the code in our tests. |
@mark-petersen Yes, it's |
@qingli411 - do you need just the magnitude of the 10-m winds? And put into what mpas-ocean field? windSpeed10m? |
@vanroekel and @qingli411: When I run the MPAS-Ocean stand alone nightly test suite with these flags:
I get a divide by zero. Adding 1e-15 to the denominator fixes it:
I’m sure it’s because the 10-meter wind field comes in as zero, so you get a divide by zero. Still, we don’t want a potential divide by zero in the code. Should I add that 1e-15 into MPAS? Or would it mess something else up? It's possible that you could have an exact zero in E3SM somewhere. |
@jonbob Yes, I only need the magnitude of the 10-meter wind speed and I was using 'windSpeed10m' |
@mark-petersen I don't see a problem adding 1e-15 to the denominator as you did. Thanks for catching that. |
Here is the new feature description page for Langmuir turbulence parameterization on Confluence: I have run a 4-year B-case simulation of this PR and compared it with the first 4 years of a previous simulation I did for the MPAS-Ocean PR #134. The results are qualitatively similar. Here are the link to MPAS-Analysis: B-case with this PR (year 1-4): First 4 year of the B-case run 20181129.Langmuir-LF17.A_WCYCL1850S.ne30_oECv3_ICG.anvil: |
agreed, adding |
Passes SMS.ne30_oECv3_ICG.A_WCYCL1850S_CMIP6.anvil_intel |
e84b461
to
6b288f8
Compare
I just rebased on today's master. @jonbob could you run a few standard tests on this branch now? @vanroekel and @qingli411 we need to fill out the rest of this page: |
@vanroekel I think you've checked this over before. Please approve this PR when you are ready. |
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.
changes and flags look good. Just need @jonbob to regenerate the buildnml bits with the auto generation scripts.
@@ -238,6 +238,9 @@ | |||
<config_cvmix_kpp_stop_OBL_search>100.0</config_cvmix_kpp_stop_OBL_search> | |||
<config_cvmix_kpp_use_enhanced_diff>.true.</config_cvmix_kpp_use_enhanced_diff> | |||
<config_cvmix_kpp_nonlocal_with_implicit_mix>.false.</config_cvmix_kpp_nonlocal_with_implicit_mix> | |||
<config_cvmix_kpp_use_theory_wave>.true.</config_cvmix_kpp_use_theory_wave> |
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.
Change to false for this PR.
@@ -238,6 +238,9 @@ | |||
<config_cvmix_kpp_stop_OBL_search>100.0</config_cvmix_kpp_stop_OBL_search> | |||
<config_cvmix_kpp_use_enhanced_diff>.true.</config_cvmix_kpp_use_enhanced_diff> | |||
<config_cvmix_kpp_nonlocal_with_implicit_mix>.false.</config_cvmix_kpp_nonlocal_with_implicit_mix> | |||
<config_cvmix_kpp_use_theory_wave>.true.</config_cvmix_kpp_use_theory_wave> | |||
<config_cvmix_kpp_langmuir_mixing_opt>'LWF16'</config_cvmix_kpp_langmuir_mixing_opt> | |||
<config_cvmix_kpp_langmuir_entrainment_opt>'LF17'</config_cvmix_kpp_langmuir_entrainment_opt> |
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.
Change to 'none' for this PR, for both of previous two.
After speaking with @vanroekel, we realized that this PR will add the relevant flags and be BFB. A later PR will be climate changing and change these three flags back:
|
6b288f8
to
4255941
Compare
@jonbob This is rebased and ready to merge. Please give it a quick test. |
Update mpas-source for Langmuir ocean vertical mixing parameterization This PR adds Langmuir mixing parameterization (via CVMix) support in mpas-ocean: * adds the option in the CVMix repository to use the 'theory-wave' approximation of Li et al., 2017 to estimate the Langmuir enhancement factor from 10-meter wind, surface friction velocity and boundary layer depth; and * adds new entries in mpaso namelist allowing different options of Langmuir mixing parameterization. The ocean source code has previously been available, but this PR adds the script and coupling changes required to use it. By default, these changes are off but this PR will be climate changing when the new flags are used to turn on the new mixing. [BFB] [NML] [FCC]
merged to next |
@mark-petersen - testing shows that 'none' in your last commit needs to be 'NONE' -- apparently it is case-sensitive. Can you add this change please? |
@mark-petersen - never mind, I'll just do it so we can be sure if works for testing tonight |
…2952) Re-merge to pick up developer error
re-merged to next. Now passes:
|
Update mpas-source for Langmuir ocean vertical mixing parameterization This PR adds Langmuir mixing parameterization (via CVMix) support in mpas-ocean: * adds the option in the CVMix repository to use the 'theory-wave' approximation of Li et al., 2017 to estimate the Langmuir enhancement factor from 10-meter wind, surface friction velocity and boundary layer depth; and * adds new entries in mpaso namelist allowing different options of Langmuir mixing parameterization. The ocean source code has previously been available, but this PR adds the script and coupling changes required to use it. By default, these changes are off but this PR will be climate changing when the new flags are used to turn on the new mixing. [BFB] [NML] [FCC]
merged to master and expected NML DIFF's blessed (but note we will expect more on cori once it's back up) |
This merge adds Langmuir mixing parameterization via CVMix
This PR is climate changing if new flags are used to turn on the new mixing.
[BFB]
[NML]
[FCC]