Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] [patch] MPI_IN_PLACE for MPI_ALLTOALL(V|W)
From: Kawashima, Takahiro (t-kawashima_at_[hidden])
Date: 2013-09-17 07:38:27


Thanks!

Takahiro Kawashima,
MPI development team,
Fujitsu

> Pushed in r29187.
>
> George.
>
>
> On Sep 17, 2013, at 12:03 , "Kawashima, Takahiro" <t-kawashima_at_[hidden]> wrote:
>
> > George,
> >
> > Copyright-added patch is attached.
> > I don't have my svn account so want someone to commit it.
> >
> > All my reported issues are in the ALLTOALL(V|W) MPI_IN_PLACE code,
> > which was implemented two months ago for MPI-2.2 conformance.
> > Not so surprising.
> >
> > P.S. Fujitsu does not yet signed the contribution agreement.
> > I must talk with the legal department again to sign it, sigh....
> > This patch is very trivial and so no issues will arise.
> >
> > Thanks,
> > Takahiro Kawashima,
> > MPI development team,
> > Fujitsu
> >
> >> Takahiro,
> >>
> >> Good catches. It's absolutely amazing that some of these errors lasted for so long before being discovered (especially the extent issue in the MPI_ALLTOALL). Please feel free to apply your patch and add the correct copyright at the beginning of all altered files.
> >>
> >> Thanks,
> >> George.
> >>
> >>
> >>
> >> On Sep 17, 2013, at 07:36 , "Kawashima, Takahiro" <t-kawashima_at_[hidden]> wrote:
> >>
> >>> Hi,
> >>>
> >>> My colleague tested MPI_IN_PLACE for MPI_ALLTOALL, MPI_ALLTOALLV,
> >>> and MPI_ALLTOALLW, which was implemented two months ago in Open MPI
> >>> trunk. And he found three bugs and created a patch.
> >>>
> >>> Found bugs are:
> >>>
> >>> (A) Missing MPI_IN_PLACE support in self COLL component
> >>>
> >>> The attached alltoall-self-inplace.c fails with MPI_ERR_ARG.
> >>> self COLL component also must support MPI_IN_PLACE.
> >>>
> >>> (B) Incorrect rcount[] index
> >>>
> >>> A trivial bug in the following code.
> >>>
> >>> for (i = 0, max_size = 0 ; i < size ; ++i) {
> >>> size_t size = ext * rcounts[rank]; // should be rcounts[i]
> >>>
> >>> max_size = size > max_size ? size : max_size;
> >>> }
> >>>
> >>> This causes SEGV or something.
> >>>
> >>> (C) For MPI_ALLTOALLV, the unit of displacements is extent, not byte
> >>>
> >>> Though the unit of displacements is byte for MPI_ALLTOALLW,
> >>> the unit of displacements is extent for MPI_ALLTOALLV.
> >>>
> >>> MPI-2.2 (page 171) says:
> >>>
> >>> The outcome is as if each process sent a message to every
> >>> other process with,
> >>> MPI_Send(sendbuf + sdispls[i] · extent(sendtype),
> >>> sendcounts[i], sendtype, i, ...),
> >>> and received a message from every other process with a call to
> >>> MPI_Recv(recvbuf + rdispls[i] · extent(recvtype),
> >>> recvcounts[i], recvtype, i, ...).
> >>>
> >>> I attached his patch (alltoall-inplace.patch) to fix these three bugs.