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

Halos - making better use of halos to improve parallel performance #220

Merged
merged 9 commits into from Mar 14, 2019

Conversation

edoddridge
Copy link
Owner

@edoddridge edoddridge commented Mar 14, 2019

As noted in #218, there are a lot of exchange calls that could be removed if the halos were being used more efficiently. This PR does just that, and partially closes #218.

There are a few bugs fixed in this PR:

  • SOR solver was using eta_star values from the halo that had never been computed. Oops. It's fixed by wrapping eta_star in that subroutine. The performance hit is irrelevant, since the internal solver is orders of magnitude slower, and can only be used on a single core.
  • diagnostic outputs did not exclude the halos when computing statistics. They do now. This required blessing new diagnostic output.

The real meat of this PR is flushing many exchange calls, which leads to substantial improvements in scaling and performance improvement when running in parallel. The 1.5 layer version is now faster when running on multiple cores than it is on a single core, which is good. The previous scaling was embarrassing.

Still to do:

  • update docs with new plots.

n-layer
red_grav

This breaks the tests (so I am knowingly commiting a broken state), but I want to show that it is the halo regions that are responsible.
Blessed output excludes halo region from the calculations.
Extend horizontal loops to make the model compute some variables inside the halos. This will remove the need for the extra exchange calls.
Also, fixes #219
Update initial blessed eta field for outcropping test
@codecov
Copy link

codecov bot commented Mar 14, 2019

Codecov Report

Merging #220 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #220      +/-   ##
==========================================
- Coverage   91.82%   91.76%   -0.06%     
==========================================
  Files          20       20              
  Lines        1822     1810      -12     
  Branches       96       95       -1     
==========================================
- Hits         1673     1661      -12     
  Misses        124      124              
  Partials       25       25
Impacted Files Coverage Δ
src/model_main.f90 96.66% <ø> (-0.06%) ⬇️
src/bernoulli.f90 100% <100%> (ø) ⬆️
src/momentum.f90 92.1% <100%> (-0.14%) ⬇️
src/aronnax.f90 94.18% <100%> (ø) ⬆️
src/barotropic_mode.f90 98.59% <100%> (ø) ⬆️
src/advection_schemes.f90 100% <100%> (ø) ⬆️
src/io.f90 94.73% <100%> (ø) ⬆️
src/thickness.f90 97.61% <100%> (-0.06%) ⬇️
src/boundaries.f90 92.77% <100%> (+0.08%) ⬆️
src/vorticity.f90 100% <100%> (ø) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50215d9...efdff6f. Read the comment docs.

@edoddridge edoddridge merged commit f7fdf64 into master Mar 14, 2019
@edoddridge edoddridge deleted the halos branch March 14, 2019 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parallel performance
1 participant