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: George Bosilca (bosilca_at_[hidden])
Date: 2013-09-17 07:35:52


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.
> <alltoall-inplace.patch>_______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel