[Mpi3-rma] current version of proposal

Fri Mar 4 21:14:16 CST 2011

Rajeev, 

Thank you very much for the comprehensive review! Comments are inline
below:

> 5:16 - "void **base" should be "void *baseptr". (void * is used in Alloc_mem)
fixed

> 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.

> 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

> 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

> 19:7 - "each buffer entry" -> "each entry in origin buffer"
> 19:12 - "each buffer entry" -> "each entry in result buffer"
> 19:19 - "each buffer entry" -> "each entry in target buffer"
done

> 19:27 -  delete * in "int *target_count"
done

> 19:37 - "datatype of" should be "origin_datatype from"
done 

> 20:23 - "the buffer entry" -> "the entry in origin, result, and target buffers"
done

> 20:42 - "datatype of" -> "datatype from"
done

> 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

> 21:13 - "of buffer entry" -> "of the element in all buffers"
done

> 21:33 - "target compare buffer" -> "target buffer"
done

> 21:34 - change to "must belong to one of the following categories of predefined datatypes"
done

> 22:42 - after "operation" add "(i.e., after the corresponding test or wait)"
done

> 25:28 - delete * in "int *target_count"
done

> 26:43 - Labeling problem: Figure 11.5 should be Figure 11.1
this seems correct in my draft?

> 26:12 - Delete the sentences from "This query functionality" onwards. There is no way to request a memory model, only to query it. So no similarity with threads.
I tried to fix it but you're right, the next sentence is enough, so I
removed it as suggested. I liked the analogy though.

> 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

> 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

> 37:14 - "refers to all" -> "refers to a lock on all"
done

> 39:20 - awkward sentence. Change to "MPI_WIN_FLUSH completes all outstanding RMA operations initiated by the calling process to the target rank. The operations are completed both at the origin and at the target." Delete sentence "RMA operations..."
done

> 40:1 - Sentence style is different from previous functions. Change to "Locally completes at the origin all..."
done

> 46:9 - Delete the part "when the RMA operation completes at the target". This Rule 5 is about when a local update is visible in the public window. It has nothing to do with RMA operation completing at target.
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.

> 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).

> 47:42 - "based on avoiding" -> "to avoid"
done

> 47:47 - "at the target" sounds like you are referring to another process, whereas you are referring to this process. Just delete it.
done

> 48:1 - Remember to update Item 1 on previous page depending on the outcome of this. Right now it contradicts Item 1.
yes, will be done later

> 49:18 - or unlock_all
done

> 50:1, Example 11.12 - Lock_all and unlock_all are not collective. They are not needed on process B. Also, I would prefer the barrier to be before local_all on process A.
Brian will fix the examples

> 50:2: Delete "Process B only needs to call lock_all..."
Brian will fix the examples

> 51:1, Example 11.14: Same. Process B does not need to call lock_all, unlock_all. I would prefer the unlock_all on process A to be before the barrier.
Brian will fix the examples

> Typos/small fixes
> -------------
> 1:39 - "in the following" -> "in this chapter"
done
> 1:47 - add comma before "which"
done
> 3:37 - delete "then"
done
> 3:45 - add comma after "libraries". Delete "then"
done
> 3:47 - add comma before "which"
done
> 5:27 - "valid," -> "valid;"
done
> 6:34 - "target. I.e," -> "target; i.e.,"
done
> 7:41 - why is "vendor" in green, should just be deleted
done
> 7:43 - "MPI; that" -> "MPI, which"
done
> 7:46 - "in" -> "on"
done
> 7:47 - "that" -> "the"
done
> 8:45 - "memory," -> "memory;"
done
> 9:2   - "This, to" -> "This is to"
done
> 9:5   - after "argument" add "to the corresponding window creation function"
done
> 9:22 - delete first word "in"
done
> 10:32 - "MPI wait" -> "MPI test or wait"
done
> 19:34 - add space between last two variables
done
> 21:2 - why is functionality in green. delete it.
done
> 26:20 - "is" should be "are" 
done
> 26:21 - which -> that
done
> 37:10 - delete green text
done
> 38:4 - E.g. -> For example   (a sentence shouldn't begin with E.g. or I.e.)
done
> 38:25 - Delete "might". Should not be in green.
done
> 39:2 - I prefer the MPI 2.2 version because "The update..." applies only to the last of the 3 options in the previous sentence.
done
> 39:23 and 25 - delete green stuff; it wasn't in 2.2.
done
> 39:24 - "document, this" -> "document, which"
done
> 44:41 - Windows -> windows
done
> 46:27 - add comma after "call"
done
> 49:31 - "is" -> "are"
I disagree, it doesn't have to delay them.
> 50:20 - "reading a" -> "reading of a"
done
> 51:18 - proces -> process
done
> 52:25 - interation -> interaction
done
> 54:48 - add comma before which
done

Thanks a lot Rajeev!

I uploaded a new version to the Wiki. This version also contains parts
of Jim's corrections. 

https://svn.mpi-forum.org/trac/mpi-forum-web/attachment/wiki/mpi3-rma-proposal1/

All the Best,
  Torsten

-- 
 bash$ :(){ :|:&};: --------------------- http://www.unixer.de/ -----
Torsten Hoefler         | Performance Modeling and Simulation Lead
Blue Waters Directorate | University of Illinois (UIUC)
1205 W Clark Street     | Urbana, IL, 61801
NCSA Building           | +01 (217) 244-7736