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

Update MPAS-O: 3d varying GM bolus and 2d varying phase speed #3057

Merged
merged 8 commits into from
Aug 30, 2019

Conversation

mark-petersen
Copy link
Contributor

@mark-petersen mark-petersen commented Jul 10, 2019

In the GM bolus calculation, there is a specified diffusivity and phase
speed (see Ferrari et al. 2010). Currently MPAS-O assumes these values
are fixed in time and space. This PR allows separate flags to enable
2D varying (+time) phase speed (based on the first baroclinic mode phase
speed) and a separate flag for 3D (+time) varying diffusivity.

See MPAS-Dev/MPAS-Model#288

[NML]
[non-BFB]

@mark-petersen
Copy link
Contributor Author

@vanroekel this is a placeholder PR. The diffs won't be correct until MPAS-Dev/MPAS-Model#288 is merged in.

Based on the test results (see MPAS-Dev/MPAS-Model#288), I think we should make vertically varying GM-Bolus default ON for all low-res runs. That would make this a non-BFB PR.

@rljacob rljacob added this to the v2.0alpha milestone Jul 11, 2019
@rljacob
Copy link
Member

rljacob commented Jul 25, 2019

@mark-petersen, I found a design document in the old Ocean Group space:
https://acme-climate.atlassian.net/wiki/spaces/OCNICE/pages/980713736/ocean+GM+Parameter+function+of+depth

But this v2 feature needs a code review document added here:
https://acme-climate.atlassian.net/wiki/spaces/ED/pages/36176475/Proposed+New+Features

And approval of the above by @golaz

@jonbob jonbob added NML non-BFB PR makes roundoff changes to answers. labels Jul 25, 2019
@vanroekel
Copy link
Contributor

@vanroekel
Copy link
Contributor

NOTE that this still needs the corresponding E3SM changes to add namelist flags and set defaults. I'll try add my changes shortly

@rljacob
Copy link
Member

rljacob commented Aug 13, 2019

@vanroekel that design document needs to be within a larger code review document. And "W16" was already taken so make it W17.

@mark-petersen mark-petersen force-pushed the mark-petersen/ocean/gmBolus3Dvarying branch from 7d2c663 to 5caa735 Compare August 16, 2019 18:18
@mark-petersen
Copy link
Contributor Author

@vanroekel this is ready for flag updates. @stephenprice this is the branch to use for your test, after Luke adds the namelist options.

@mark-petersen
Copy link
Contributor Author

mark-petersen commented Aug 16, 2019

FYI, just rebased this on current master, so it should work on cori, but I haven't tested it.

flags are added to enable the 3D varying GM bolus Kappa, it is enabled
by default and is on whenever the GM parameterization is on.

output of kappaGM3D and the phase speed are added to monthly history as
well
@vanroekel
Copy link
Contributor

@stephenprice the branch is now ready for you to test.

@mark-petersen
Copy link
Contributor Author

Passes these tests on cori-knl:

PASS PEM_Ln9.ne30_oECv3_ICG.A_WCYCL1850S.cori-knl_intel COMPARE_base_modpes
PASS PET_Ln3.T62_oEC60to30v3wLI.GMPAS-DIB-IAF-ISMF.cori-knl_intel COMPARE_base_single_thread
PASS PET_Ln9.T62_oEC60to30v3.GMPAS-IAF.cori-knl_intel COMPARE_base_single_thread

So I know the updated namelists work. Thanks @vanroekel and @jonbob

@mark-petersen
Copy link
Contributor Author

OK, I updated the design document here. Added performance test and reviewed the rest, and filled in the meta-data information at the top that is collected in the page one level up.
https://acme-climate.atlassian.net/wiki/spaces/ED/pages/1023836492/W17+MPAS-Ocean+3D+varying+GM+bolus+kappa+Design+Document

@jonbob let's proceed with this one.

@mark-petersen
Copy link
Contributor Author

@vanroekel please approve this PR.

@golaz, I think we are asking for team lead approval, and this falls under the water cycle. I think you just need to acknowledge that we filled out everything on the design document, see last post.

Copy link
Contributor

@vanroekel vanroekel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good, just requesting two variables be added to the streams file by default.

@mark-petersen
Copy link
Contributor Author

@jonbob I added those two variables, so all requests are completed.

@jonbob
Copy link
Contributor

jonbob commented Aug 22, 2019

@mark-petersen - what was that last commit? This PR doesn't say anything about bringing in a new grid and doesn't have the CIME support for it

@mark-petersen
Copy link
Contributor Author

Whoops - working on two things at once. I'll fix that, thanks for looking.

@mark-petersen mark-petersen force-pushed the mark-petersen/ocean/gmBolus3Dvarying branch from a89b783 to 6ff481a Compare August 22, 2019 14:43
@mark-petersen
Copy link
Contributor Author

OK, just rolled back the last commit. This is the only thing I added for Luke's requested variables:
6ff481a

Copy link
Contributor

@vanroekel vanroekel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look good. Thanks @mark-petersen

jonbob added a commit that referenced this pull request Aug 29, 2019
Update MPAS-O: 3d varying GM bolus and 2d varying phase speed

n the GM bolus calculation, there is a specified diffusivity and phase
speed (see Ferrari et al. 2010). Currently MPAS-O assumes these values
are fixed in time and space. This PR allows separate flags to enable
2D varying (+time) phase speed (based on the first baroclinic mode phase
speed) and a separate flag for 3D (+time) varying diffusivity.

See MPAS-Dev/MPAS-Model #288

[NML]
[non-BFB]
@jonbob
Copy link
Contributor

jonbob commented Aug 29, 2019

merged to next

jonbob added a commit that referenced this pull request Aug 30, 2019
Update MPAS-O: 3d varying GM bolus and 2d varying phase speed

In the GM bolus calculation, there is a specified diffusivity and phase
speed (see Ferrari et al. 2010). Currently MPAS-O assumes these values
are fixed in time and space. This PR allows separate flags to enable
2D varying (+time) phase speed (based on the first baroclinic mode phase
speed) and a separate flag for 3D (+time) varying diffusivity.

See MPAS-Dev/MPAS-Model #288

[NML]
[non-BFB]
@jonbob jonbob merged commit b9f84d3 into master Aug 30, 2019
@jonbob
Copy link
Contributor

jonbob commented Aug 30, 2019

merged to master and expected DIFF's blessed

@jonbob jonbob deleted the mark-petersen/ocean/gmBolus3Dvarying branch August 30, 2019 14:23
@wlin7 wlin7 mentioned this pull request Sep 5, 2019
mark-petersen added a commit to MPAS-Dev/MPAS-Model that referenced this pull request Oct 5, 2019
This PR fixes a threading issue that was introduced in [E3SM PR#3057]
(E3SM-Project/E3SM#3057) that brought in
the initial implementation of 3D varying GM bolus. After that commit,
some E3SM threading tests were observed to fail with different results.

Tested with:
* SMS_P12x2.T62_oQU240.CMPASO-NYF.compy_intel - BFB results 10 times
* SMS_P12x2.ne4_oQU240.A_WCYCL1850.compy_intel.allactive-mach_mods - BFB
* results 10 times

This last test is one that was failing on sandiatoss3.
jonbob added a commit that referenced this pull request Oct 7, 2019
…3226)

Fix threading issue in MPAS-O GM routine

This PR brings in a new mpas-source submodule that fixes a threading issue that
was introduced in E3SM PR #3057. After that commit, which brought in the initial
implementation of 3D varying GM bolus, some E3SM threading tests were observed
to fail with different results.

Tested with:
* SMS_P12x2.T62_oQU240.CMPASO-NYF.compy_intel - BFB results 10 times
* SMS_P12x2.ne4_oQU240.A_WCYCL1850.compy_intel.allactive-mach_mods - BFB results 10 times
This last test is one that was failing on sandiatoss3.

Fixes #3200
[BFB]
jonbob added a commit that referenced this pull request Oct 8, 2019
Fix threading issue in MPAS-O GM routine

This PR brings in a new mpas-source submodule that fixes a threading issue that
was introduced in E3SM PR #3057. After that commit, which brought in the initial
implementation of 3D varying GM bolus, some E3SM threading tests were observed
to fail with different results.

Tested with:
* SMS_P12x2.T62_oQU240.CMPASO-NYF.compy_intel - BFB results 10 times
* SMS_P12x2.ne4_oQU240.A_WCYCL1850.compy_intel.allactive-mach_mods - BFB results 10 times
This last test is one that was failing on sandiatoss3.

Fixes #3200
[BFB]
jonbob added a commit that referenced this pull request Jun 11, 2020
…(PR #3632)

Adding E3SM support for 3DGM namelist options to maint-1.2

This adds changes from PR #3057 for E3SM support of 3DGM namelist options to the
maint-1.2 branch. As part of this, 3DGM is turned on by default (as it was in
PR #3057), so it is not BFB. The changes were copied directly from the E3SM (non
mpas-source) file changes in #3057 (hash 9fc9d6f).

[non-BFB]
[NML]
ashwathsv pushed a commit to ashwathsv/MPAS-Model that referenced this pull request Jul 21, 2020
…elop

This PR fixes a threading issue that was introduced in [E3SM PR#3057]
(E3SM-Project/E3SM#3057) that brought in
the initial implementation of 3D varying GM bolus. After that commit,
some E3SM threading tests were observed to fail with different results.

Tested with:
* SMS_P12x2.T62_oQU240.CMPASO-NYF.compy_intel - BFB results 10 times
* SMS_P12x2.ne4_oQU240.A_WCYCL1850.compy_intel.allactive-mach_mods - BFB
* results 10 times

This last test is one that was failing on sandiatoss3.
jgfouca pushed a commit that referenced this pull request Nov 7, 2024
…-orbital-parameter-type

Automatically Merged using E3SM Pull Request AutoTester
PR Title: Fix type in orbital parameter read
PR Author: brhillman
PR LABELS: radiation, AT: AUTOMERGE, bugfix, AT: Skip weaver, AT: Skip v1 Testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mpas-ocean NML non-BFB PR makes roundoff changes to answers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants