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 for halo exchange reuse #2041

Closed

Conversation

mark-petersen
Copy link
Contributor

This PR speeds up the ocean model by storing and reusing halo exchange data structures, rather than recompute them on every halo exchange. This is only applied to the barotropic subcycle in this PR, which has the largest number of halo exchange calls. Tests have shown an ocean speed-up of 8-12% on cori-knl with threading, but it should benefit all architectures and thread configurations.
[BFB]

@mark-petersen
Copy link
Contributor Author

Thanks to @Larofeticus for continued work on MPAS performance, and @ndkeen for collaboration and testing within E3SM.

@mark-petersen
Copy link
Contributor Author

The added MPAS framework subroutines are already in E3SM, from this MPAS PR:
MPAS-Dev/MPAS#1453
More discussion on the ocean changes are here:
MPAS-Dev/MPAS#1458

In MPAS-Ocean stand alone, this passed all tests in nightly regression suite. BFB with previous commit using both gnu/opt and intel/debug and between 1 and 2 threads. Tested in E3SM with:

PEM_Ln9.T62_oQU240.GMPAS-IAF.cori-knl_intel
SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.theta_intel
SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.cori-knl_intel

@jonbob jonbob self-assigned this Jan 19, 2018
jonbob added a commit that referenced this pull request Jan 19, 2018
…2041)

Update MPAS-O for halo exchange reuse

This PR speeds up the ocean model by storing and reusing halo exchange
data structures, rather than recompute them on every halo exchange. This
is only applied to the barotropic subcycle in this PR, which has the
largest number of halo exchange calls. Tests have shown an ocean
speed-up of 8-12% on cori-knl with threading, but it should benefit all
architectures and thread configurations.

Tested with:
* PEM_Ln9.T62_oQU240.GMPAS-IAF.cori-knl_intel
* SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.theta_intel
* SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.cori-knl_intel

[BFB]
@jonbob
Copy link
Contributor

jonbob commented Jan 19, 2018

merged to next

@jonbob
Copy link
Contributor

jonbob commented Jan 23, 2018

Off-line testing by @pwolfram showed a severe memory leak with this version. I ran a test of GMPAS-IAF.T62_oECv3 on anvil and it failed with memory issues after a month and a half -- so this need to be reverted until fixed.

@ndkeen
Copy link
Contributor

ndkeen commented Jan 23, 2018

Jon: I was also noticing that we had a few failures on next related to memory leak. I was hoping to ignore them, but apparently not!

FYI:

edison and cori-haswell both see the following tests fails due to memory leak:
SMS.T62_oQU120_ais20.MPAS_LISIO_TEST
SMS_P12x2.ne4_oQU240.A_WCYCL1850

and then cori-knl sees the same failure with SMS_P12x2.ne4_oQU240.A_WCYCL1850, but for some reason passes on the other one.

@mark-petersen
Copy link
Contributor Author

Reverted MPAS-O commit due to insufficient memory error. See MPAS-Dev/MPAS#1490
This PR needs to be on hold until the memory leak is fixed.

jonbob added a commit that referenced this pull request Jan 23, 2018
…ext (PR #2041)"

This reverts commit ec6757b, reversing
changes made to d502dd1.
@jonbob
Copy link
Contributor

jonbob commented Jan 24, 2018

@ndkeen - thanks for the note. It does add some confidence that the automated testing does catch egregious memory leaks like this

@mark-petersen mark-petersen deleted the mark-petersen/ocean/halo_exchange_reuse branch January 28, 2018 18:13
jgfouca pushed a commit that referenced this pull request Jun 1, 2018
New gx3v7_coastal grid file

Updated the namelists for `runoff_to_ocn` to use a new grid file for the coastal
mask of the gx3v7 grid (old grid file had different area than full gx3v7 grid
file); after generating new grid files, I also updated the cesm defaults to use
the new maps instead of the old / broken ones.

Test suite: Ran a single C compset using the `rx1 -> gx3v7` maps, verified that answers changed slightly (have not tested the `r05 -> gx3v7` map)
Test baseline: Same case as above, but run off `master` instead of my branch
Test namelist changes: `ROF2OCN_LIQ_RMAPNAME`, `ROF2OCN_ICE_RMAPNAME`, `GLC2OCN_LIQ_RMAPNAME` and `GLC2OCN_ICE_RMAPNAME` change when running CESM with `gx3v7` ocean grid
Test status: roundoff

Fixes #2041 

User interface changes?: nope

Update gh-pages html (Y/N)?: nope

Code review: none
wlin7 pushed a commit that referenced this pull request Feb 26, 2023
Improve qv_sat to allow more caller info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB enhancement mpas-ocean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants