Reorder ranks to improve locality in communication

Description

Right now, all ranks are distributed in a 4D hypercube where the dimensions are domain, states, k points, and other (in this order). The call to MPI_Cart_create returns a cartesian communicator where usually all ranks are assigned in a row-major way. This means that the last index runs fastest (other) and the first index runs slowest (domain).

Usually, tasks are assigned to nodes in a block fashion, i.e. the first ranks (0 to x-1) go the first node, the ranks x to 2x-1 go to the second node etc. This means that the last parallelization strategy that is used is as compact as possible, whereas the first parallelization strategy is always spread out over several nodes. This means that currently the domain parallelization is not compact, i.e. the ranks sharing the domain for the same state and k point are usually spread out over several nodes.

The problem here is that communication inside a node is faster than across nodes because it can be handled in the shared memory as opposed to going through the interconnect. Thus, the communication that happens most often should be between ranks that are as compact as possible (to avoid going through the interconnect).

In octopus, the communication that happens most often is that using the domain parallelization strategy. Thus, it should be beneficial for most applications to have a compact arrangement of the tasks in the domain parallelization. This can be achieved by reordering the ranks in the base communicator in a way that reflects the column-major order of the 4D hypercube.

Tests have shown up to 20% speed-up depending on the input data. For best performance, set ParDomains to the number of cores per node (or a fraction/multiple of this number, e.g. 1/2 or 1/4 or 2).

News snippet

Reorder ranks to improve locality in communication

Checklist

• I have checked that my code follows the Octopus coding standards
• I have added tests for all the new features added in this request.

changed milestone to %10.0

added Core Optimization labels

Codecov Report

Merging #733 into develop will increase coverage by <.01%. The diff coverage is 77.77%.

@@             Coverage Diff             @@
##           develop     #733      +/-   ##
===========================================
+ Coverage    70.25%   70.26%   +<.01%     
===========================================
  Files          482      482              
  Lines        93102    93128      +26     
===========================================
+ Hits         65412    65432      +20     
- Misses       27690    27696       +6

Impacted Files	Coverage Δ
src/basic/multicomm.F90	86.71% <77.77%> (-0.72%)

---

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 e01e0ae...be84dd9. Read the comment docs.

added 1 commit

• a5fb8a9e - Reorder ranks to improve locality in communication

Compare with previous version

added 1 commit

• 81228f58 - Reorder ranks to improve locality in communication

Compare with previous version

added 2 commits

• 2f28eacb - Make reordering the ranks an option
• dffcc591 - Add test for reordering feature

Compare with previous version

added 1 commit

• 769ba63d - Change order of tests

Compare with previous version

marked the checklist item I have added tests for all the new features added in this request. as completed

unmarked as a Work In Progress

mentioned in merge request !434 (closed)

assigned to @micael.oliveira

@sohlmann From MR !434 (closed), it seems that there are cases where the option to reorder the ranks will not work. I think it is quite dangerous to have an option that sometimes causes a calculation to fail without even really knowing why or when this will happen.

added 69 commits

• 769ba63d...e01e0ae0 - 63 commits from branch develop
• 90c93d74 - Reorder ranks to improve locality in communication
• f14863d6 - Make reordering the ranks an option
• 15aa92de - Add test for reordering feature
• b707c0ae - Change order of tests
• c96fd0c7 - Use reordered ranks for base group
• be84dd9d - Remove experimental status

Compare with previous version

So I rebased on develop and added the two commits to make the testsuite pass, as in !739 (closed).

As discussed, I am not making this option default for now, because it is not yet clear that it is always beneficial.

mentioned in merge request !739 (closed)

approved this merge request

resolved all threads

enabled an automatic merge when the pipeline for be84dd9d succeeds

merged

mentioned in commit 5e4db7fd