[Mpi3-rma] current version of proposal

Tue Mar 8 13:48:34 CST 2011

I believe that this has already been fixed in some version of the doc  
- I just downloaded the current version, and this has been fixed  
already.

Bill

On Mar 8, 2011, at 1:44 PM, James Dinan wrote:

> Hi All,
>
> I think there's a bug in the prototype for MPI_Get_accumulate:
>
> int MPI_Get_accumulate(void *origin_addr, int origin_count,
>   MPI_Datatype origin_datatype, void *result_addr,
>   int result_count, MPI_Datatype result_datatype,
>   int target_rank, MPI_Aint target_disp, int *target_count,
>   MPI_Datatype target_datatype, MPI_Op op, MPI_Win win)
>
> I think that the target count should be 'int' rather than 'int*'.
> MPI_Rget_accumulate has this same type problem for the origin, result,
> and target counts:
>
> int MPI_Rget_accumulate(void *origin_addr, int *origin_count,
>   MPI_Datatype origin_datatype, void *result_addr,
>   int *result_count, MPI_Datatype result_datatype,
>   int target_rank, MPI_Aint target_disp, int *target_count,
>   MPI_Datatype target_datatype, MPI_Op op, MPI_Win win,
>   MPI_Request *req)
>
> Cheers,
>  ~Jim.
>
> On 3/5/11 4:45 PM, Torsten Hoefler wrote:
>> On Fri, Mar 04, 2011 at 11:35:41PM -0600, Rajeev Thakur wrote:
>>> Thanks. A few more comments:
>>>
>>>>> 5:28 - What discussion of alloc_mem in Section 8.2 also applies  
>>>>> here? Is it the Rationale on the next page after free_mem? That  
>>>>> should be mentioned here. It is important to mention here that  
>>>>> void* actually means void**.
>>>> Yes, it's now referencing discussions and retionale.
>>>
>>> I don't think the current sentence "The discussion of and  
>>> rationales for MPI_ALLOC_MEM and MPI_FREE_MEM in Section 8.2 also  
>>> apply to MPI_WIN_ALLOCATE"
>>>
>>> is enough for the user to realize there is important information  
>>> back in Sec 8.2 that explains that void *baseptr is actually void  
>>> **baseptr. And it's a common mistake users make when passing this  
>>> parameter. How about adding this text "in particular... " at the  
>>> end:
>>>
>>> "The discussion of and rationales for MPI_ALLOC_MEM and  
>>> MPI_FREE_MEM in Section 8.2 also apply to MPI_WIN_ALLOCATE; in  
>>> particular, see the rationale in Section 8.2 for an explanation of  
>>> the type used for \mpiarg{baseptr}.
>> done
>>
>>>>> 6:11 - Change "This is combined with local routines" to "It must  
>>>>> be used in combination with local routines (MPI_Win_attach and  
>>>>> MPI_Win_detach)"
>>>> done
>>>
>>> Can you delete the parentheses around those functions (although they
>>> were there in my mail).
>> done - I also added a "the"
>>
>>>>> 7:19 - Awkward sentence. Change to "The memory region specified  
>>>>> must not contain any part that is already attached to the window  
>>>>> win." Delete the part starting from "that is"
>>>> done
>>>
>>> Change "are erroneous" to "is erroneous" in 7:21.
>> done
>>
>>>>> 19:37 - "datatype of" should be "origin_datatype from"
>>>> done
>>>
>>> Bad line break now at 19:37.
>> Hmm, that's a macro thing :-(. Not sure what to do.
>>
>>>>> 20:46 - awkward sentence. Change "and MPI_NO_OP or MPI_REPLACE"  
>>>>> to "as well as MPI_NO_OP and MPI_REPLACE," (add comma at the end)
>>>> done
>>>
>>> There should be a comma after MPI_REDUCE as well (as was there  
>>> earlier).
>> done
>>
>>>>> 29:43 - These two calls ->  These four calls. Add sentence to  
>>>>> cover lock_all and unlock_all
>>>> done
>>>>
>>>>> 31:35 - comma after "needed". Delete then
>>>>> 31:36   - make it compare-and-swap
>>>> done
>>>
>>> The last one is not done. I had suggested changing "compare and  
>>> swap and accumulates" to "compare-and-swap and accumulates" (i.e.,  
>>> adding the hyphens) just to make the sentence read more clearly.
>> this is gone now
>>
>>>>> 37:11 - Awkward sentence. Change to "During the epoch, the  
>>>>> calling process can access the window memory on all processes in  
>>>>> win by using RMA operations."
>>>> done
>>>
>>> win should be \mpiarg{win}
>> done
>>
>>>>> 40:1 - Sentence style is different from previous functions.  
>>>>> Change to "Locally completes at the origin all..."
>>>> done
>>>
>>> Typo: origina ->  origin
>> done
>>
>>> I would prefer "Locally completes" instead of "Locally complete".  
>>> It is a short form of "This function locally completes..."
>> done
>>
>>>>> 47:14 - I would say delete this advice to implementors. If we  
>>>>> keep it, I would like to fix the first sentence, which is awkward.
>>>> I fixed it.
>>>
>>> "Overlapping accesses (and other operations that MPI-3 specifies)  
>>> are undefined."
>>> It is not clear to the reader what "other operations" the text in  
>>> parenthesis refers to.
>> right, I removed it.
>>
>>> "However, implementations may wish to provide a mode in which such  
>>> operations are erroneous to aid in debugging code."
>>>
>>> "are erroneous to aid in debugging code" at the end of the  
>>> sentence doesn't read well. I would change that sentence to
>>>
>>> "However, to assist users in debugging code, implementations may  
>>> wish to provide a mode in which such operations are erroneous."
>> done
>>
>>>>> 47:28 - why is "that use the same operation" being removed. It  
>>>>> should be there.
>>>> no, this is now in the new info key. I added a sentence  
>>>> clarifying this
>>>> (and referencing back).
>>>
>>> OK. With that deletion, the chapter doesn't say anywhere whether  
>>> concurrent overlapping accumulates with different operators are  
>>> allowed or not. Or are they allowed only if one of the two  
>>> operations is a no_op?
>>>
>>> And what is the default if the user doesn't pass any info key  
>>> accumulate_ops? What should the implementation assume?
>> Right, I believe Brian is working on a fix (we should put this on the
>> list of open issues).
>>
>> A new version is uploaded.
>>
>> Thanks&  All the Best,
>>   Torsten
>>
>
> _______________________________________________
> mpi3-rma mailing list
> mpi3-rma at lists.mpi-forum.org
> http://lists.mpi-forum.org/mailman/listinfo.cgi/mpi3-rma

William Gropp
Deputy Director for Research
Institute for Advanced Computing Applications and Technologies
Paul and Cynthia Saylor Professor of Computer Science
University of Illinois Urbana-Champaign