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

Add normalGMBolusVelocity to the gm package #5167

Conversation

mark-petersen
Copy link
Contributor

@mark-petersen mark-petersen commented Sep 1, 2022

This PR is needed to put the normalGMBolusVelocity in the gm package, so it is not allocated when GM is off. Otherwise, extra memory is allocated unnecessarily for high-resolution simulations.

This reverts commit 2c6f496, which was a small change in #5099 to make that PR BFB on intel-19 optimized in MPAS-Ocean standalone.

This PR is BFB with gcc-9 debug, gcc-9 optimized, and intel-19 debug. For intel-19 optimized there is a mismatch with master of 1e-12 on the first timestep for the MPAS-Ocean standalone compass test ocean_global_ocean_QU240_PHC_performance_test on crysalis. It appears that intel-19 conducts some optimization that changes the order of operations for the lines involved here.

We expect this PR to be bfb for all E3SM tests.

[BFB]

@mark-petersen mark-petersen added mpas-ocean non-BFB PR makes roundoff changes to answers. labels Sep 1, 2022
@mark-petersen
Copy link
Contributor Author

Ran the nightly suite again to confirm:

This PR is BFB with gcc-9 debug, gcc-9 optimized, and intel-19 debug. For intel-19 optimized there is a mismatch with master of 1e-12 on the first timestep for the compass test ocean_global_ocean_QU240_PHC_performance_test

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.

Approving by visual inspection — comparison with the commit in #5099 that removed these changes.

@jonbob
Copy link
Contributor

jonbob commented Sep 6, 2022

test merge passes:

  • SMS_Ld1.ne30pg2_EC30to60E2r2.WCYCL1850.chrysalis_intel.allactive-wcprod
  • SMS_D_Ld1.ne30pg2_EC30to60E2r2.WCYCL1850.chrysalis_intel.allactive-wcprod
  • SMS_D_Ld3.T62_oQU120.CMPASO-IAF.chrysalis_intel
  • ERS.ne11_oQU240.WCYCL1850NS.chrysalis_intel
  • SMS_P12x2.ne4_oQU240.WCYCL1850NS.chrysalis_intel.allactive-mach_mods

@mark-petersen -- we may need to rework the PR in reference to it being non-BFB. Maybe state that there are potential order-of-operations differences possible, but not found in testing on chrysalis?

@mark-petersen mark-petersen changed the title Ocean Submesoscale Parameterization: Minor nonBFB change Add normalGMBolusVelocity to the gm package Sep 6, 2022
@mark-petersen mark-petersen added BFB PR leaves answers BFB and removed non-BFB PR makes roundoff changes to answers. labels Sep 6, 2022
@mark-petersen
Copy link
Contributor Author

@jonbob I updated the first PR comment to be more detailed description of the purpose of this PR, and how the non-bfb result occurs.

jonbob added a commit that referenced this pull request Sep 6, 2022
…5167)

Add normalGMBolusVelocity to the gm package

This PR is needed to put the normalGMBolusVelocity in the gm package, so
it is not allocated when GM is off. Otherwise, extra memory is allocated
unnecessarily for high-resolution simulations.

This reverts commit 2c6f496, which was a small change in #5099 to make
that PR BFB on intel-19 optimized in MPAS-Ocean standalone.

This PR is BFB with gcc-9 debug, gcc-9 optimized, and intel-19 debug.
For intel-19 optimized there is a mismatch with master of 1e-12 on the
first timestep for the MPAS-Ocean standalone compass test
ocean_global_ocean_QU240_PHC_performance_test on chrysalis. It appears
that intel-19 conducts some optimization that changes the order of
operations for the lines involved.

We expect this PR to be bfb for all E3SM tests.

[BFB]
@jonbob
Copy link
Contributor

jonbob commented Sep 6, 2022

Thanks @mark-petersen - I think that's much more clear

Merged to next

@jonbob jonbob merged commit ecd8efc into E3SM-Project:master Sep 7, 2022
@jonbob
Copy link
Contributor

jonbob commented Sep 7, 2022

merged to master and nonBFB results blessed on compy-pgi

@mark-petersen mark-petersen deleted the mark-petersen/ocn/revert-nonbfb-submeso branch September 9, 2022 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB mpas-ocean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants