Update MPAS components and framework, bring in threading optimizations #1630

jonbob · 2017-07-11T20:59:14Z

This PR updates all MPAS submodules and framework, including:

bfb framework threading optimization from Abhinov MPAS#1237;
bfb mpas-o threading optimization from Abhinov MPAS#1235;
update path to new Albany install (cherry-picked from branch agsalin/machines/AlbanyNewInstall);
various framework and component updates.

Tested with:

PET_Ln9.T62_oEC60to30v3.GMPAS-NYF.edison_intel
SMS.T62_oEC60to30v3.GMPAS-IAF.edison_intel (bfb with current master)
PET.T62_oEC60to30v3.GMPAS-NYF.titan

Note: this commit is answer-changing only for compsets with MPASLI
[non-BFB]

A new MPAS Landice tag, to be pulled into ACME in the next few weeks, will have an updated interface to Albany/FELIX. I have started to upgrade the Albany installation on targeted machines. In this commit, we update the path in the ALBANY_PATH variable in config_compiler.xml for some machines. This makes that change for redsky/chama, edison, and sandia-srn-sems machines. ToDo: install new albany on Mira, Titan, Anvil, and other machines. The hope is that the test MPASLIALB in acme_integration will pass with this change on these machines when done in concert with the MPAS Landice update.

…ovements.

matthewhoffman

@jonbob , I visually looked over the changes, and they all look good to me - I don't see anything amiss. I did not do any actual testing.

philipwjones

the threading PR appears to be missing two mods:

in mpas_ocn_time_integration_rk4.F: remove the omp master directives around the vector reconstruct - the biggest change in this mod is a threaded reconstruction so these master directives are disabling the threaded reconstruction

in maps_ocn_diagnostics.F: similarly, the omp sections directives around various reconstructs should also be removed for similar reasons

Not sure how these got dropped. All other mods looked consistent with what I tested previously. Suspect it's still ok to merge since rk4 is never used in ACME and the other will likely do no harm. But would prefer they be fixed now. Thx.

jonbob · 2017-07-12T21:11:59Z

Thanks @philipwjones - those were from the mpas-o commit, so I'll ask @mark-petersen to take a look

mark-petersen · 2017-07-13T05:09:17Z

OK, I remerged that commit back in. @philipwjones please look at this MPAS commit:
https://github.com/mark-petersen/MPAS/tree/ocean/remove_omp_master_directives
Which is your old commit merged back in again:
https://github.com/mark-petersen/MPAS/commit/c09b5c7087fb6d78b81c6d0b5368e90022ab6f44
Does that look right?
@jonbob I updated the MPAS-O commit on this PR, so if @philipwjones agrees, then this is ready to be tested again.

jonbob · 2017-07-13T14:01:12Z

Thanks @mark-petersen - I'm testing again on edison. But will also wait for @philipwjones review

jonbob · 2017-07-13T19:04:24Z

The PET test still passes -- just waiting on a smoke test to make sure results are bob with previous master as well.

jonbob · 2017-07-13T21:35:09Z

OK, the smoke test on edison also completed and passed the compare with the same run on master. I'll work on getting this into the repo

Update MPAS components and framework, bring in threading optimizations This PR updates all MPAS submodules and framework, including: * bfb framework threading optimization from Abhinov MPAS#1237; * bfb mpas-o threading optimization from Abhinov MPAS#1235; * update path to new Albany install (cherry-picked from branch agsalin/machines/AlbanyNewInstall); * various framework and component updates. Tested with: * PET_Ln9.T62_oEC60to30v3.GMPAS-NYF.edison_intel * SMS.T62_oEC60to30v3.GMPAS-IAF.edison_intel (bfb with current master) * PET.T62_oEC60to30v3.GMPAS-NYF.titan [BFB]

jonbob · 2017-07-24T20:30:34Z

merged to next

This reverts commit e410be4, reversing changes made to 6207121. Conflicts: cime/config/acme/machines/config_compilers.xml

mark-petersen · 2017-07-26T12:41:51Z

Just FYI for all, this PR used a new process for testing the MPAS-O submodule. Normally, we test an MPAS-O commit and merge to MPAS-Dev repo, ocean/develop branch and mirror it to ACME-Climate repo, ocean/develop branch. Those branches are both considered permanent. The problem is, if there are any issues in the ACME PR testing due to MPAS-O, it is then inconvenient to modify the MPAS-O commits.

For this PR, we instead pushed a test branch of MPAS-O to the ACME-Climate repo (https://github.com/ACME-Climate/MPAS/tree/acme/mark-petersen/ocean_develop_test_branch_170622) and test this ACME PR with that submodule commit. After this PR has been merged into ACME, I will push the test branch to the head of the MPAS-Dev/ocean/develop repo. Alternately, I could merge it into MPAS-Dev/ocean/develop if there have been other recent merges. The important thing is that the specific commit that the ACME submodule points to remains in the permanent repository.

A bit of git acrobatics. Thanks to @matthewhoffman for discussing this process.

The previous update of MPASLI in this branch brought in changes that were not BFB for the configuration used in ACME. In fixing that bug, I also discovered that the configuration in ACME was not bit-reproducible. So this update also fixes that bug. However, that fix makes this update answer-changing. Differences are small, and the new results are identical across different processor counts. I also update the namelist option to force the Albany mesh to be rebuilt on every time step. This is needed to allow Albany runs to be BFB-restartable until a fix in Albany is brought into ACME. [Non-BFB]

matthewhoffman · 2017-07-31T17:41:33Z

@jonbob , I just pushed a new commit to fix the bugs in MPASLI.

rljacob · 2017-08-03T16:24:04Z

@jonbob go ahead and put this back on next even if its "closed".

matthewhoffman · 2017-08-03T16:26:32Z

(@jonbob , we'll have to remember to revert the revert before re-merging, as I explained on the ACME integrators' guide last time we were in this situation.)

jonbob · 2017-08-03T16:33:32Z

Thanks @rljacob - on it

…#1630)"" This reverts commit 23080db. Conflicts: cime/config/acme/machines/config_compilers.xml

Update MPAS components and framework, bring in threading optimizations This PR updates all MPAS submodules and framework, including: * bfb framework threading optimization from Abhinov MPAS#1237; * bfb mpas-o threading optimization from Abhinov MPAS#1235; * update path to new Albany install (cherry-picked from branch agsalin/machines/AlbanyNewInstall); * various framework and component updates. Tested with: * PET_Ln9.T62_oEC60to30v3.GMPAS-NYF.edison_intel * SMS.T62_oEC60to30v3.GMPAS-IAF.edison_intel (bfb with current master) * PET.T62_oEC60to30v3.GMPAS-NYF.titan Note: this commit is answer-changing only for compsets with MPASLI [non-BFB]

jonbob · 2017-08-03T21:23:31Z

merged to next

rljacob · 2017-08-05T04:47:54Z

This PR is still producing diffs on next (using the melvin results) for these tests:

SMS.f09_g16_a.IGCLM45_MLI.melvin_gnu
SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.melvin_gnu
ERS.f09_g16_g.MPASLISIA.melvin_gnu

One test also has namelist diffs:
ERS_Ld5.T62_oQU120.CMPASO-NYF.melvin_gnu
BASE: config_write_output_on_startup = .true.
COMP: config_write_output_on_startup = .false.
Differences in namelist 'am_watermasscensus':
found extra variable: 'config_am_watermasscensus_compute_predefined_regions'
found extra variable: 'config_am_watermasscensus_region_group'

rljacob · 2017-08-07T18:33:39Z

I see that the description was updated to note that MPASLI cases are non-BFB and acme_developer testing confirmed that. This can go ahead to master.

jonbob · 2017-08-07T18:38:07Z

Thanks -- I'll get that handled right away

jonbob · 2017-08-07T18:38:29Z

Do you want to wait for a clean testing of next, or should I merge to master now?

rljacob · 2017-08-07T18:47:31Z

merge now. Have no idea when we'll get a clean run of integration on next. If there's unseen problems, integration testing on master will catch it.

jonbob · 2017-08-07T18:55:50Z

Thanks

Update MPAS components and framework, bring in threading optimizations This PR updates all MPAS submodules and framework, including: * bfb framework threading optimization from Abhinov MPAS#1237; * bfb mpas-o threading optimization from Abhinov MPAS#1235; * update path to new Albany install (cherry-picked from branch agsalin/machines/AlbanyNewInstall); * various framework and component updates. Tested with: * PET_Ln9.T62_oEC60to30v3.GMPAS-NYF.edison_intel * SMS.T62_oEC60to30v3.GMPAS-IAF.edison_intel (bfb with current master) * PET.T62_oEC60to30v3.GMPAS-NYF.titan Note: this commit is answer-changing only for compsets with MPASLI [non-BFB]

jonbob · 2017-08-07T20:28:46Z

merged to master

ndkeen · 2017-08-08T20:14:22Z

fwiw, I checked this out yesterday and ran a high res G case on cori-knl with 1 and 2 threads per MPI. I don't have numbers from the same experiment before the change (can get that). Just wanted to check that there were no obvious issues when trying it with 2 threads.

jonbob · 2017-08-08T20:24:10Z

That's excellent news -- thanks @ndkeen

mark-petersen · 2017-08-08T20:26:40Z

Yes, thanks @ndkeen for the extra test. That is good to hear.

Update MPAS components and framework, bring in threading optimizations This PR updates all MPAS submodules and framework, including: * bfb framework threading optimization from Abhinov MPAS#1237; * bfb mpas-o threading optimization from Abhinov MPAS#1235; * update path to new Albany install (cherry-picked from branch agsalin/machines/AlbanyNewInstall); * various framework and component updates. Tested with: * PET_Ln9.T62_oEC60to30v3.GMPAS-NYF.edison_intel * SMS.T62_oEC60to30v3.GMPAS-IAF.edison_intel (bfb with current master) * PET.T62_oEC60to30v3.GMPAS-NYF.titan Note: this commit is answer-changing only for compsets with MPASLI [non-BFB]

agsalin and others added 3 commits July 6, 2017 12:32

Update MPAS framework and components, including MPAS-O threading impr…

c369705

…ovements.

Also include corresponding mpas-o threading changes

36d359c

jonbob added BFB PR leaves answers BFB mpas-albany-landice mpas-ocean mpas-seaice labels Jul 11, 2017

jonbob self-assigned this Jul 11, 2017

jonbob requested a review from matthewhoffman July 11, 2017 20:59

jonbob mentioned this pull request Jul 11, 2017

Update MPAS submodules to test threading improvements: framework, BFB #1345

Closed

rljacob mentioned this pull request Jul 12, 2017

Correct MG2 rain number conservation limiter #1599

Merged

jonbob requested a review from philipwjones July 12, 2017 19:22

matthewhoffman approved these changes Jul 12, 2017

View reviewed changes

philipwjones requested changes Jul 12, 2017

View reviewed changes

Update mpas-ocean commit to remove OMP master diretives

9aa04f6

philipwjones approved these changes Jul 13, 2017

View reviewed changes

rljacob added this to the v1.0beta2 milestone Jul 17, 2017

jonbob added a commit that referenced this pull request Jul 25, 2017

Revert "Merge branch 'jonbob/mpas/update-070617' into next (PR #1630)"

23080db

This reverts commit e410be4, reversing changes made to 6207121. Conflicts: cime/config/acme/machines/config_compilers.xml

jonbob added a commit that referenced this pull request Aug 3, 2017

Revert "Revert "Merge branch 'jonbob/mpas/update-070617' into next (PR …

b3d848e

…#1630)"" This reverts commit 23080db. Conflicts: cime/config/acme/machines/config_compilers.xml

jonbob added non-BFB PR makes roundoff changes to answers. and removed BFB PR leaves answers BFB labels Aug 5, 2017

jonbob merged commit c9b1e61 into master Aug 7, 2017

jonbob deleted the jonbob/mpas/update-070617 branch August 7, 2017 20:28

This was referenced Aug 7, 2017

Threading optimization: bfb framework changes by Abhinav MPAS-Dev/MPAS#1237

Merged

Threading optimization: bfb ocean changes by Abhinav MPAS-Dev/MPAS#1235

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update MPAS components and framework, bring in threading optimizations #1630

Update MPAS components and framework, bring in threading optimizations #1630

jonbob commented Jul 11, 2017 •

edited

Loading

matthewhoffman left a comment

philipwjones left a comment

jonbob commented Jul 12, 2017

mark-petersen commented Jul 13, 2017

jonbob commented Jul 13, 2017

jonbob commented Jul 13, 2017

jonbob commented Jul 13, 2017

jonbob commented Jul 24, 2017

mark-petersen commented Jul 26, 2017

matthewhoffman commented Jul 31, 2017

rljacob commented Aug 3, 2017

matthewhoffman commented Aug 3, 2017

jonbob commented Aug 3, 2017

jonbob commented Aug 3, 2017

rljacob commented Aug 5, 2017

rljacob commented Aug 7, 2017

jonbob commented Aug 7, 2017

jonbob commented Aug 7, 2017

rljacob commented Aug 7, 2017

jonbob commented Aug 7, 2017

jonbob commented Aug 7, 2017

ndkeen commented Aug 8, 2017

jonbob commented Aug 8, 2017

mark-petersen commented Aug 8, 2017

Update MPAS components and framework, bring in threading optimizations #1630

Update MPAS components and framework, bring in threading optimizations #1630

Conversation

jonbob commented Jul 11, 2017 • edited Loading

matthewhoffman left a comment

Choose a reason for hiding this comment

philipwjones left a comment

Choose a reason for hiding this comment

jonbob commented Jul 12, 2017

mark-petersen commented Jul 13, 2017

jonbob commented Jul 13, 2017

jonbob commented Jul 13, 2017

jonbob commented Jul 13, 2017

jonbob commented Jul 24, 2017

mark-petersen commented Jul 26, 2017

matthewhoffman commented Jul 31, 2017

rljacob commented Aug 3, 2017

matthewhoffman commented Aug 3, 2017

jonbob commented Aug 3, 2017

jonbob commented Aug 3, 2017

rljacob commented Aug 5, 2017

rljacob commented Aug 7, 2017

jonbob commented Aug 7, 2017

jonbob commented Aug 7, 2017

rljacob commented Aug 7, 2017

jonbob commented Aug 7, 2017

jonbob commented Aug 7, 2017

ndkeen commented Aug 8, 2017

jonbob commented Aug 8, 2017

mark-petersen commented Aug 8, 2017

jonbob commented Jul 11, 2017 •

edited

Loading