Hi Jeff, as with all reviews, I've put it into http://www.hlrs.de/mpi/mpi21/doc/MPI-2.1draft-2008-02-23-review.txt User / Password as usual: mpi21 / mpi21 Most of your proposals in Binding, I commented with R2 to give it back to you for implementing it. Best regards Rolf On Mon, 7 Apr 2008 11:18:32 -0700 Jeff Squyres <jsquyres_at_[hidden]> wrote:

While reading the April 1 MPI-2.1 document for C++ and chapter 13 issues, I itemized a list of other issues that I found. Some may want to be pushed off to 2.2 (I have noted all of these with "--> This will require a vote") -- I leave these up to the Forum to decide -- but others should be fixed in 2.1.

Enjoy.

-----

April 7 2008 Comments on MPI-2.1 April 1, 2008 document

(notation: pX.Y = page X, line Y)

Overall =======

- Frontage numbering is still roman numerals -- I thought we agreed to have the physical and PDF page numbers agree.

- Some function pointer typedefs are "_function", others are "_fn" We should be consistent throughout the text.

Intro chapter =============

- p5.2: Two open source MPI implementations are cited that are no longer relevant. This entire sentence should be removed. Indeed, the language in the overall paragraph is forward-looking -- it should probably be re-worked to be in the present tense.

- p6.46: Chapter 13 discusses Fortran issues (not Fortran 90 issues), and also discusses language interoperability issues.

- p7.11: Looks like a broken LaTeX reference.

Terms and Conventions =====================

- p9.40-10.10: The C and Fortran language neutral name do not include the prefix "MPI_", but the C++ bindings include the prefix "MPI::". Shouldn't the C/Fortran bindings be "MPI_CLASS_ACTION_SUBSET" (etc.)?

- p10.35: due to the decision from last meeting (sort out the IN/OUT/INOUT mess in MPI-2.2), the language should be softened in this paragraph and the full paragraph following this one (because they contradict each other). Specifically, I propose changing:

10.35: Thus, in C++, IN aguments are either references... to Thus, in C++, IN arguments are usually either references...

- p11.22: "Fortran in this document refers to Fortran 90". For MPI-2.1, it is probably suitable to leave this, but we might want to make a statement (footnote or parenthetical) that it is expected to be updated in future MPI spec revisions.

Point-to-Point Communication ============================

- p66.33: Why is the request argument to the C bindings MPI_CANCEL take a pointer? The language neutral binding is IN, and the C++ binding is const. This seems to be a contradiction. --> This cannot be changed easily because every MPI implementation uses (MPI_Request *) as the type.

- p83.29: extra space: "( int count,"

- p92.43: mising "const" before "void* location". Adding "const" will not break any user codes or MPI implementations. --> This will require a vote.

- p95.47-48: remove "const" before lb an extent arguments. Removing "const" will not break any user codes or MPI implementations. --> This will require a vote.

Collective Communications =========================

- p123.32: "relaxed" uses quote marks in the wrong direction

- p123.27: should say "MPI-1" instead of "MPI-1.2"

Groups, Contexts, and Communicators ===================================

- p186.5: Descriptions for the C++ bindings need to be included here, since they are different than the C bindings. The text from 13.1.7 would seem to be sufficient.

- p214.32: Missing "const" before "void* attribute_val_in" (datatype callback argument is const) --> This will require a vote.

- p214.36: Delete function comm argument should be const (?) --> This will require a vote.

- p219.5: Missing "const" before "void* attribute_val_in" (datatype callback argument is const) --> This will require a vote.

- p219.9: Delete function win argument should be const (?) --> This will require a vote.

- p221.44: Delete function type argument should be const (?) --> This will require a vote.

MPI Environmental Management ============================

- p248.3: Earlier in the document, references are made to v1.3. Is that not a valid tuple to return from MPI_GET_VERSION?

- p249.45: Remove duplicate C++ binding

- p254.21: Remove erroneous MPI::Intracomm::Create C++ binding

- p255.2: First argument (comm handle reference) to errhandler function should be const. --> This will require a vote.

- p256.27: First argument (win handle reference) to errhandler function should be const. --> This will require a vote.

- p257.41: First argument (file handle reference) to errhandler function should be const. --> This will require a vote.

- p258.39: Delete duplicate C++ binding

- p259.13: Delete duplicate C++ binding

- p260.36-43: First letter of description should be capitolized to be consistent with rest of list.

- p261.42: Delete duplicate C++ binding

- p263.21-23: Delete duplicate C++ bindings

- p264.22: Delete duplicate C++ binding

- p267.24: Delete duplicate C++ binding

- p267.42: Delete duplicate C++ binding

Process Creation and Management ===============================

- p288.11: Function should be const. --> This will require a vote.

- p288.15: Function should be const. --> This will require a vote.

External Interfaces ===================

- p344.39: Arguments of query_fn, free_fn, and cancel_fn need to have *'s. This is a consistency issue (the rest of the standard uses "*" for function pointer parameters), not a correctness issue -- works both ways. Should not break any user code or MPI implementations.

I/O ===

- p419.43: datatype argument should be const

Language Bindings =================

- p437.18: Delete first sentence of paragraph

- p437.22: Delete "original"

- p437.24-29: Delete first and last sentences of paragraph

- p437.29: New last sentence, "See Annex A.4.19 for a function cross reference."

- p438.11: Delete "for MPI-1"

- p438.14-37: Join the two namespaces together and delete the "Additionally, the following classes defined for MPI-2:" text. Re-sort the class names as appropriate.

- p439.2: Delete last sentence ("To maintain ... in [23].")

- p442: Table 13.1 is redundant with Annex A.1. However, the following C++ types are missing from Annex A.1:

MPI::SIGNED_CHAR MPI::BOOL MPI::COMPLEX MPI::DOUBLE_COMPLEX MPI::LONG_DOUBLE_COMPLES

- p442: Table 13.2 is redundant with Annex A.1

- p445.5-11: These functions are not listed in Annex A

- p445.36-38: Remove this entire paragraph ("Compilers that do not support..."). This feature has been a part of C++ since C++98, and exists in all modern C++ compilers.

- p446.7-17: Declarations for the MPI::Exceptions class are missing from Annex A

- p448.47: Remove trailing "/" from "OUT/"

- p450.26-44: The bullet list is wholly redundant with section 2.6.2.

- p456.27-29: VOLATILE is in Fortran 200x; reword first sentence from: "In the longer term, the attribute VOLATILE is under consideration for Fortran 2000 and would give the buffer or variable the properties needed, but it would inhibit optimization of any code containing the buffer or variable." to "The VOLATILE attribute, available in later versions of Fortran, would give the buffer or variable the properties needed, but it would inhibit optimization of any code containing the buffer or variable."

- p459.11-15: Delete paragraph

- p462.43-45: Delete paragraph

- p463.17-18: Delete first sentence; delete "In MPI-2,"

- p467.23: There is an extra space before "MPI_FINALIZE"

- p468.16-39: None of these functions appear in Annex A (they should!)

- p469.41 and 44: These functions do not appear in Annex A (they should!)

- p470.46: Change sentence from: "It is important to note that all C++ objects and their corresponding C handles..." to "It is important to note that all C++ objects with corresponding C handles..." I.e., this sentence is referring only to C++ handles that actually have corresponding C handles (e.g., not MPI::ERRORS_THROW_EXCEPTIONS).

Profiling Interface ===================

- p481.4: Delete "const" --> This may require a vote.

Annex A =======

- p494.15: s/Fortran/Fortran only

- p494.30: Missing MPI::ERRORS_THROW_EXCEPTIONS

- p495.18-19 and 33-34: MPI_BYTE and MPI_PACKED are listed in two separate tables.

- p495.30: MPI::COMPLEX should be MPI::F_COMPLEX

- p495.38: Looks like a blank extra 3rd column is in this table

- p495.bottom: Missing table for "C++ only" datatypes: MPI::BOOL MPI::COMPLEX MPI::DOUBLE_COMPLEX MPI::LONG_DOUBLE_COMPLEX MPI::SIGNED_CHAR

- p496.1: Add ", no Fortran" before the ")"

- p496.10, s/Fortran/Fortran only/

- p496.27-30: Alignment of table entries should be to the left, not centered.

- p544.13: Many of the prototypes for Clone() and Dup() are missing

- p547.all: Many doubles of C++ bindings; likely because of errors in the chapter text.

- p554.41: Do we need an empty deprecated functions section?

- p555.45-46: Forgot to mention MPI_ERRCODES_IGNORE, MPI_F_STATUSES_IGNORE, and MPI_F_STATUS_IGNORE

- p556.bottom: Missing prototypes for the MPI::Exceptions class

-- Jeff Squyres Cisco Systems

_______________________________________________ mpi-21 mailing list mpi-21_at_[hidden] http://lists.mpi-forum.org/mailman/listinfo.cgi/mpi-21

Dr. Rolf Rabenseifner . . . . . . . . . .. email rabenseifner_at_[hidden] High Performance Computing Center (HLRS) . phone ++49(0)711/685-65530 University of Stuttgart . . . . . . . . .. fax ++49(0)711 / 685-65832 Head of Dpmt Parallel Computing . . . www.hlrs.de/people/rabenseifner Nobelstr. 19, D-70550 Stuttgart, Germany . (Office: Allmandring 30)