MPI Communication

[sslattery]

Adds support for MPI and basic communication plans and operations:

1. CommunicationPlan - base class for all communication plans. Contains the base implementation which determines the number of imports and exports as well as neighbor ranks based on export data.

2. Distributor - provides a communication plan for migrate to move data from one uniquely-owned distribution to another uniquely-owned distribution. This derives from CommunicationPlan.

3. Halo - provides a communication plan for gather and scatter to manipulate ghosted data. This also derives from CommunicationPlan

Each has a decent set of unit tests that caught some bugs. I have also tested this with a GPU-aware OpenMPI implementation although we are currently only using CUDA UVM (we will be adding regular CUDA memory soon).

[codecov-io]

Codecov Report

Merging #43 into master will decrease coverage by 1.3%.
The diff coverage is 96.3%.

@@           Coverage Diff            @@
##           master     #43     +/-   ##
========================================
- Coverage    99.2%   97.8%   -1.4%     
========================================
  Files          12      15      +3     
  Lines         379     731    +352     
========================================
+ Hits          376     715    +339     
- Misses          3      16     +13

Impacted Files	Coverage Δ
core/src/Cabana_Distributor.hpp	93.5% <93.5%> (ø)
core/src/Cabana_Halo.hpp	95.8% <95.8%> (ø)
core/src/Cabana_CommunicationPlan.hpp	99.1% <99.1%> (ø)

---

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 ea3c5eb...4579770. Read the comment docs.

[sslattery]

@dalg24 @junghans @sjplimp @rfbird This one is finally ready for review. I am not opposed to splitting this PR into 3 PRs - one for each of the 3 new classes. Please take a look when you have a chance. Feedback on the APIs would be useful.

[dalg24]

TLDR

I only looked at CommunicationPlan so far.

Why are CommunicationPlan:: createFromExportsAndTopology(), :: createFromExportsOnly(), and :: createExportSteering() public member functions? The first two feel like they should be constructors, and the last one should be marked as protected if you are only going to call it from derived classes.

Similarly why did you chose Distributor:: createFromExportsAndNeighbors() and :: createFromExports() over two constructor overloads?

[dalg24]

Const-correctness: Either return Kokkos::View<std::size_t const*,kokkos_memory_space> or drop the const qualifier on the function member.

[dalg24]

Also this is really confusing to give an accessor (getter with no side effect) such a name.

[sslattery]

I have changed the name as we discussed

[sslattery]

@dalg24 I refactored the inheritance structure here. The base class now has functions that are essentially protected but they must be made public to use class data with CUDA. I changed the create functions in the derived classes as well. This makes it clear that an entirely new communication plan is being created.

[dalg24]

Some more feedback. I looked at Distributor and Halo.
It would have been nice to generalize/reuse loops that start the nonblocking receives and that perform the blocking sends.

[sslattery]

@junghans What do you think is the best way here to enable CommunicationPlan, Distributor, Halo, and their tests/examples only when MPI is enabled?

[dalg24]

Say I create a new header and forget to list it here. Will it be caught when building and running the tests?

[sslattery]

It may although that is tough to tell because it is header only so it may just pull it in. It will likely show up more when external libraries link in and the missing header is not installed.

[junghans]

No, it won’t, hence globbing and subfolders might be preferred!

[sslattery]

Lets push this as is with regards to the build system them and move this conversation to #55 - once we come to a conclusion there we can implement in a new PR

[sslattery]

Things to finish:

• Fix name of exportSteering() -> getExportSteering()

• Fix name of numExportElement() -> exportSize()

• Add MPI_Barrier to all communication functions to avoid potential race conditions for same MPI tag calls and the add async versions in the future