Re: [Mpi3-rma] Fwd: [Mpi-comments] Comments on MPI 3.0 draft 2 [part 4]
Hi Bill, As you apply these edits, could you filter out the feedback that should be discussed by the working group? Thanks, ~Jim. On 8/23/12 4:55 PM, William Gropp wrote:
For those of you not on the mpi-comments list, we have been receiving a very detailed set of comments from Jeremiah Willcock. He's made it to the RMA chapter (#11); this message also include chapter 12. Please share comments on these; for the obvious typos, I as chapter author will repair them as ticket-0 changes.
Bill
William Gropp Director, Parallel Computing Institute Deputy Director for Research Institute for Advanced Computing Applications and Technologies Paul and Cynthia Saylor Professor of Computer Science University of Illinois Urbana-Champaign
Begin forwarded message:
*From: *Jeremiah Willcock <[email protected] <mailto:[email protected]>> *Subject: **[Mpi-comments] Comments on MPI 3.0 draft 2 [part 4]* *Date: *August 23, 2012 4:45:03 PM CDT *To: *<[email protected] <mailto:[email protected]>> *Reply-To: *Comments on the MPI Standard <[email protected] <mailto:[email protected]>>
These are in reference to the draft at <URL:http://meetings.mpi-forum.org/mpi3.0_draft_2.pdf>. Thank you for considering them.
-- Jeremiah Willcock
Chapter 11:
Why are the request-based one-sided functions named MPI_R* rather than MPI_I* like nonblocking point-to-point and collective functions?
There are no request-based versions of fetch-and-op or compare-and-swap.
Page 403 line 31: Add a comma after the second capital "B".
Page 403 line 34: The semicolon here can be changed to a comma, and probably removed entirely.
Page 404 line 15: "distributed memory" should be hyphenated.
Page 404 line 22: The first comma should be a colon.
Page 404 line 23: There should be a comma after "MPI_WIN_ALLOCATE_SHARED".
Page 404 line 25: There is extra space around "to specify".
Page 404 line 30: Should MPI be in sans-serif font?
Page 405 line 24: Remove "and C++".
Page 405 line 32: "address sized" should be hyphenated.
Page 405 lines 42-43; page 408, lines 26-27; page 409, line 47: Which of these info keys must be set consistently between different processes?
Page 406 lines 4-9: Can accumulate_ops be validly and portably set to anything other than the two values listed there?
Page 406 line 17: There should be a comma after "units".
Page 406 lines 42-43: "implementation specific" should be hyphenated.
Page 409 line 44: The function name on the next line looks like it might fit on this one.
Page 411 lines 4-5: Window flavors have not yet been introduced. In particular, the allocation functions should probably say which flavor of window they each produce.
Page 411 lines 7-8: Some of these argument names might need to be in sans-serif.
Page 411 lines 30-: "RMA" was written in sans-serif earlier in the chapter.
Page 412 line 34: "MPI" should be in sans-serif.
Page 413 line 21: "win" should be in sans-serif.
Page 413 lines 21-22: Can the same memory be attached to different windows, just not the same window?
Page 414 line 25: The "should" might need to be "must" to avoid issues similar to <URL:https://upc-bugs.lbl.gov/bugzilla/show_bug.cgi?id=495>.
Page 415 line 12: "called" should be "call".
Page 416 line 6: There should be commas after "disp_unit" and "create_kind".
Page 416 line 20: There should be a comma after "returned". What if the user modifies the pointees of the pointers that are the values of these attributes in C?
Page 416 line 28: Why return the group rather than the communicator itself? The group returned is listed as a "duplicate" -- does that mean the user is responsible for freeing it?
Page 417 line 2: "MPI" should be in sans-serif.
Page 417 line 12: "this" should be "the".
Page 418 line 17: There should be a comma after "MPI_RGET_ACCUMULATE".
Page 418 line 27: Make sure to note that "locally completed" does not mean the same thing as "completed" (I think). Will those be defined in detail later?
Page 418 line 30: There should be a comma after "get call".
Page 418 line 35: "; namely," can be changed to just a colon.
Page 418 line 36: Which calls count as "accumulate calls"?
Page 420 line 2: There should be a comma after "origin node".
Page 420 line 31: "shared memory" should be hyphenated.
Page 420 line 40: "occurred" should be "occurs".
Page 422 line 7: There should be a comma after "B".
Page 423 lines 31 and 37, page 424 lines 18 and 24: Would it be helpful to users to show the correct hints ("assert" arguments) to use in MPI_WIN_FENCE to express each fence's semantics to the implementation?
Page 423 line 47: The example header should be on the same page as the example text.
Page 424 line 33: I think "contribution" should be "contributions".
Page 425 line 40: There should be a comma after "origin_count".
Page 426 lines 9-11: Is there a valid reason that MPI_REPLACE (and MPI_NO_OP) are not allowed for MPI_REDUCE? They are not useful, most likely, but orthogonality seems to suggest that they be allowed anyway.
Page 426 line 18: There should be a comma after "B".
Page 426 line 46: Remove "then".
Page 428 line 15: There is a line break (and thus probably a space in the source code) after the left parenthesis here.
Page 428 line 26: "and MPI_NO_OP or" should be "MPI_NO_OP, or".
Page 429: Is there a reason that there is not a similar special-case version of MPI_ACCUMULATE?
Page 430: What happens if an MPI implementation does not support, for example, compare-and-swap on MPI_INTEGER16 values?
Page 430 line 21: Please put compare_addr as its own variable declaration to avoid a line break between two variables.
Page 430 line 48: "passive-target epoch" has not been defined yet as far as I know, and this use of the term does not reference a definition.
Page 431 line 10: There should be a comma after "MPI_WIN_FLUSH_LOCAL".
Page 432 line 15: There should be a comma after "MPI_WIN_UNLOCK".
Page 433: Is it possible/likely that an rget does not complete locally until after a flush, fence, or similar call? That might be worth mentioning if true.
Page 436 lines 24-25: "RMA" here should be in sans-serif (plus italic).
Page 436 lines 26-34: This seems to imply sequential consistency between get and put accesses on the same window, even if they are not consistent with loads and stores? Or is that issue removed by the rules about overlapping get and put accesses in the same epoch?
Page 436 lines 35-39: Can different processes see the updates in a different order (than each other's view of them, not just from the order the updates actually occurred)?
Page 437: "passive target" and "active target" should be hyphenated.
Page 437 line 19: "shared memory" should be hyphenated.
Page 438 line 7: There should be a comma after "MPI_WIN_POST".
Page 441: Does MPI_WIN_FENCE start and/or end active-target or passive-target epochs? This is answered on page 438, but the answer should be here as well.
Page 443 line 13: There should be a space between the comma and "win".
Page 443 line 22: "occurred" should be "occurs".
Page 443 line 25: Insert "has" before "called".
Does it make sense to have a nonblocking version of MPI_WIN_START? What about MPI_WIN_FENCE or the other synchronization calls that could block (MPI_WIN_WAIT already has a nonblocking version, although it does not use an MPI_Request).
Page 445 line 21: "call" should be "calls".
Page 445 lines 25 and 31: The "no need to wait" part is not a complete sentence.
Page 445 lines 24-34: "initiate" should be either "initiates" or capitalized in each of these.
Page 446 line 2: The tuple representing the graph should probably use \langle and \rangle rather than < and >; this will probably fix the spacing around them as well.
Page 446 lines 6 and 10: "group" in each of these lines might need to be sans-serif.
Page 447 line 48: There is extra space before "load".
Page 448 line 1: "call" should be "calls".
Page 448 line 3: "lock protected" should be hyphenated.
Page 450 lines 16-18: How does this affect whether rget, rput, ... accesses have "locally completed"? Is this sentence true about those kinds of accesses until MPI_Wait is called on them? Is MPI_Wait on such a request guaranteed not to block after MPI_WIN_FLUSH_LOCAL returns?
Page 450 line 46: "copy" should be "copies"?
Page 451 line 15: "implementation" should be "implementation's".
Page 451 line 17: "assertion specific" should be hyphenated.
Page 451 line 25: There should be a comma after "MPI_MODE_NOPRECEDE".
Page 452 lines 1 and 9: There is extra space before "stores" on each of these lines.
Page 452 line 22: There should be a comma after "acquire".
Page 452 line 32: "argument" should be "arguments".
Page 452 line 43: "comm" should be in sans-serif.
Page 453 lines 33-36: Is this text defining "complete", or just repeating a definition stated officially elsewhere? If it is the definition, the wording doesn't make that obvious.
Page 454 lines 14 and 21: "becomes visible" means "becomes visible eventually" (in an implementation-defined amount of time); that may be worth stating explicitly here.
Page 454 line 18: There is a space missing before "MPI_WIN_LOCK".
Page 454 line 29: Insert "a" or "the" before "private". Which process does "the process memory" refer to?
Page 454 lines 35-36: What is the motivation for putting "even" before "flush"?
Page 454 line 47: This needs to be qualified with "within the same access epoch" or similar, like the next sentence is.
Page 455 line 4: "MPI" should be sans-serif.
Page 455 lines 26 and 29: Does "update" mean remote update here?
Page 456 line 1: "will" should be "would".
Page 456 line 18: "a remote process" should be plural.
Page 456 line 23: "magnitude" should be "magnitudes".
Page 456 line 38: There is extra space before "updates".
Page 456 line 41: Add "elsewhere" before "in".
Page 456 line 45: Remove "in the case".
Page 457 line 14: There is extra space before "stores".
Page 457 line 20: Should "being posted" be "posted" instead? The comma after "posted" can be removed.
Page 457 line 47: "implementation dependent" should be hyphenated.
Page 458 line 5: "X" (and probably "A" and "B" where they appear) should be in code font. This applies to the rest of the examples as well.
Page 458 lines 11 and 17, page 459 lines 4 and 10, page 460 line 35, and page 461 line 9: "EXCLUSIVE" should be "MPI_LOCK_EXCLUSIVE".
Page 458 line 23: There is extra space after "combining".
Page 458 line 26: "the" should be "an".
Page 458 line 38: There is extra space after "not".
Page 458 line 39: "compiler and hardware specific" should be "compiler- and hardware-specific".
Page 461 line 27: How are get-accumulate and similar operations incorporated into these restrictions? It would be very useful to be able to use fetch-and-add to get unique counter values, for example, although this operation will return different answers based on message interleavings (contradicting lines 36 and 37).
How does compare-and-swap fit into the rules about accumulate_ops? Is compare-and-swap considered to be another reduction op for the purposes of the concurrent update rules, or is it a special case?
Page 461 line 30: The comma should be a semicolon or colon.
Page 462: Is it possible to express the different ordering models in terms of shared-memory consistency models (sequential, cache-coherent, etc.)? Those names are easier to look up than MPI-specific terms for accumulate_ordering values.
Page 462 lines 31-32: Is this true for both memory models, or is there a stronger rule for the unified memory model?
Page 463 lines 28 and 31: Remove the comma after "reversed".
Page 463 line 30: Remove the comma after "deadlock".
[may not be fixable] Page 464 lines 29-30: It looks odd to have "end of advice to users" after something saying that the entire section is advice to users.
Page 464 line 32: "value" should be "values".
Page 465 line 8: There is extra whitespace in this line.
Page 465 lines 11-19: Section titles are in sans-serif here, while they are normal text in the rest of the standard.
Page 465 line 43: "use" and "provide" should be "uses" and "provides", respectively.
Page 467 line 26: There should be a space before "*/".
Page 468 line 4: The semicolon should be a comma.
Page 468 line 7: "naive" might need to be 'na\"\ive'.
Page 469 lines 16 and 19: "A" as the variable name might need to be in the code font.
Page 469 lines 38-39: "NSTEPS" and "M" should be in the code font, or else "NSTEPS" needs to be inside \mathit.
Page 470 line 25: "N" might need to be in the code font or math mode.
Chapter 12:
Page 476 line 42: There is extra space after "request".
There are a few places where "thread safe" is used as an adjective; in that context, it should be spelled "thread-safe".
Page 487: I think MPI_INIT_THREAD should be moved to be near MPI_INIT in the standard, since many high-performance applications use threads and any threading at all (even OpenMP in non-MPI-using regions) is forbidden when MPI_INIT is used.
[post 3.0] In the future, it might make sense for MPI_INIT to choose a thread mode such as MPI_THREAD_FUNNELED (or for implementations to be allowed to choose one) as the default when, for example, OpenMP is enabled by compiler settings. Many people are probably using MPI_INIT in OpenMP applications without knowing that MPI makes doing that erroneous.
Page 488 lines 37-41: The text in lines 39-41 seems to contradict the behavior of MPI_INIT required by 37-38.
Page 489 line 22: "is specified at link time." is repeated twice.
It might be worthwhile to discuss MPI_MPROBE and MPI_IMPROBE and why they are necessary in threaded contexts in "advice to users" or similar non-normative text somewhere in this chapter. _______________________________________________ mpi-comments mailing list [email protected] <mailto:[email protected]> http://lists.mpi-forum.org/mailman/listinfo.cgi/mpi-comments
_______________________________________________ mpi3-rma mailing list [email protected] http://lists.mpi-forum.org/mailman/listinfo.cgi/mpi3-rma
participants (1)
-
Jim Dinan