open-mpi / ompi

Open MPI main development repository
https://www.open-mpi.org
Other
2.14k stars 859 forks source link

MPI_Comm_split_type does not treat 'key' correctly #8854

Closed bangerth closed 3 years ago

bangerth commented 3 years ago

Thank you for taking the time to submit an issue!

Background information

What version of Open MPI are you using? (e.g., v3.0.5, v4.0.2, git branch name and hash, etc.)

2.1.1 (packaged with Ubuntu 18.04)

Describe how Open MPI was installed (e.g., from a source/distribution tarball, from a git clone, from an operating system distribution package, etc.)

Via Ubuntu 18.04

Please describe the system on which you are running


Details of the problem

MPI describes MPI_Comm_split_type as follows (copy pasting from the MPI 3.1 documentation):

int MPI_Comm_split_type(MPI_Comm comm, int split_type, int key, MPI_Info info, MPI_Comm *newcomm)

This function partitions the group associated with comm into disjoint subgroups, based on the type specified by split_type. Each subgroup contains all processes of the same type. Within each subgroup, the processes are ranked in the order defined by the value of the argument key, with ties broken according to their rank in the old group. A new communicator is created for each subgroup and returned in newcomm. This is a collective call; all processes must provide the same split_type, but each process is permitted to provide different values for key. An exception to this rule is that a process may supply the type value MPI_UNDEFINED, in which case newcomm returns MPI_COMM_NULL.

The emphasis is mine. I read this as follows: The rank within the new sub-communicator should be determined by the lexicographic ordering of the pairs (key, rank-in-old-comm) -- with first preference given to the key and second preference to the old rank. But this isn't happening, as shown in the following program where I want rank=1 in MPI_COMM_WORLD to become rank=0 in the sub-communicator:

#include <mpi.h>

#define CHECK_MPI(ierr) if (ierr!=0) std::abort();

unsigned int this_mpi_process(const MPI_Comm &mpi_communicator) {
  int       rank = 0;
  const int ierr = MPI_Comm_rank(mpi_communicator, &rank);
  CHECK_MPI(ierr);

  return rank;
}

int main(int argc, char *argv[]) {
  int ierr;

  ierr = MPI_Init(&argc, &argv);
  CHECK_MPI(ierr);

  const MPI_Comm communicator = MPI_COMM_WORLD;

  const int key = (this_mpi_process(communicator) == 1 ? 0 : 1);
  MPI_Comm sub_comm;
  ierr = MPI_Comm_split_type(communicator,
                             MPI_COMM_TYPE_SHARED,
                             key,
                             MPI_INFO_NULL,
                             &sub_comm);
  CHECK_MPI(ierr);

  std::cout << "Global rank "
            << this_mpi_process(communicator)
            << " has key "
            << key
            << " and has sub-comm rank "
            << this_mpi_process(sub_comm)
            << std::endl;

  ierr = MPI_Finalize();
  CHECK_MPI(ierr);
}

When run with mpirun -np 3 ./test, I get this output:

Global rank 0 has key 1 and has sub-comm rank 0
Global rank 1 has key 0 and has sub-comm rank 1
Global rank 2 has key 1 and has sub-comm rank 2

The final number should have been 1, 0, 2 instead.

Interestingly, though, while I can't seem to sort a rank to the front by choosing a small key, I can sort a rank to the end by choosing a large key. If I set

 const int key = (this_mpi_process(communicator) == 1 ? 1000 : 1);

then I get this output:

Global rank 0 has key 1 and has sub-comm rank 0
Global rank 1 has key 1000 and has sub-comm rank 2
Global rank 2 has key 1 and has sub-comm rank 1

One might suspect that the ordering of keys is turned around (largest keys get highest final ranks), but I really don't know whether that conjecture is true.

In any case, this came up in the context of https://github.com/dealii/dealii/pull/12022 .

bangerth commented 3 years ago

Interestingly, the problem is not in MPI_Comm_split: If I call

 ierr = MPI_Comm_split(communicator,
                        /*color=*/0,
                        key,
                        &sub_comm);

I get as expected

Global rank 0 has key 1 and has sub-comm rank 1
Global rank 1 has key 0 and has sub-comm rank 0
Global rank 2 has key 1 and has sub-comm rank 2
jsquyres commented 3 years ago

Thanks for the report! A fix is coming in #8861 on master, and then we'll bring it back to the release branches.

bangerth commented 3 years ago

Ha, how fun -- a one-character fix! Thanks for the quick patch, @jsquyres !

peterrum commented 3 years ago

Thanks a lot, indeed!