coll/basic: fix neighbor alltoall message ordering #7063
Conversation
This commit updates the coll/basic component to correctly order sends and receives for cartesian communicators with cyclic boundaries. This addresses an issue identified by mpi-forum/mpi-issues#153. This issue occurs when the size in any dimension is 1. This gives the same neighbor in the positive and negative directions. The old code was sending and receiving in the same order so the -1 buffer contained the +1 result and vise-versa. The problem is addressed by using unique tags for each send. This should cover both the case where overtaking is allowed and is not allowed. The former case will be possible is a MPI_Cart_create_with_info() call is added to the standard. Signed-off-by: Nathan Hjelm <hjelmn@google.com>
|
This doesn't fix the non-blocking variant. There may be additional issues with the other neighbor collectives. |
|
@bosilca Can you please review. |
|
First, the problem exists also for 2 processes in a direction (not only for 1). |
|
@RolfRabenseifner Indeed. It can happen with 1-2 processes in a direction. Will update the commit message. As for MPI_Cart_create_with_info, the way the allow overtaking rule is implemented we would run into the same issue if we fixed the issue just by re-ordering the receives. Using tags covers that case as well. |
|
Be very careful: Two nonblocking MPI_NEIGHBOR_ALLTOALL still need for the same tags that the messages from process to process must not overtake. |
| nreqs++; | ||
| rc = MCA_PML_CALL(irecv(rbuf, rcount, rdtype, srank, | ||
| MCA_COLL_BASE_TAG_ALLTOALL, | ||
| MCA_COLL_BASE_TAG_NONBLOCKING_BASE - 2 * dim, |
There was a problem hiding this comment.
This tag range was reserved (as the name indicates) for nonblocking collectives. If we are using it for something else (as we should in this particular case) I would suggest we rename it to something more meaningful.
There was a problem hiding this comment.
Yeah. We can add another reserved range to be safe. I can add that to this PR.
|
@RolfRabenseifner, as indicated in Nathan's comment this patch only applies to blocking collectives. |
|
@bosilca Yup. The non-blocking fix is much more complicated. This is low hanging fruit. |
|
@hjelmn we might need to have more well-defined ranges for collectives, and have a sliding window of tags to prevent the issues with concurrent collectives. |
|
This PR was replaced by #7232. Closing without merging. |
4 participants
This commit updates the coll/basic component to correctly order sends
and receives for cartesian communicators with cyclic boundaries. This
addresses an issue identified by mpi-forum/mpi-issues#153. This issue
occurs when the size in any dimension is 1. This gives the same
neighbor in the positive and negative directions. The old code was
sending and receiving in the same order so the -1 buffer contained
the +1 result and vise-versa. The problem is addressed by using
unique tags for each send. This should cover both the case where
overtaking is allowed and is not allowed. The former case will be
possible if a MPI_Cart_create_with_info() call is added to the
standard.
Signed-off-by: Nathan Hjelm hjelmn@google.com