Re: [Mpi-forum] First reading: MPI_T / Tools Chapter
Hi Martin, Here is round two of my review (the second half of the proposal). First, in yesterday's email, I suggested the following change: p 12, line 13 Change "This routine returns, if datatpye represents a valid enumeration datatype, N representing the range of the enumeration 0 to N-1 as well as a string with a name for it." "The arguments name and ..." to "If datatype is a valid enumeration datatype, this routine returns the number of items in the enumeration range and the name of the datatype. If the range runs from 0 to N-1, the value N is returned in num. The arguments name and ..." But now I think a better way to write this is: "If the argument datatype is a valid enumeration datatype, this routine returns the number of items in the enumeration range and the name of the datatype. The number of items, N, is returned in the argument num. If N > 0, then the enumeration range runs from 0 to N-1. If N = 0, then the enumeration range is empty. The arguments name and ..." Maybe we don't need to worry about N=0? Alright, now here are comments for the second half of the document. p 16, line 43 "local to a single MPI process" --> "local to an MPI process" p 16, line 46 "the interface has a cleaner semantics" --> "the interface provides cleaner semantics" p 17, line 2 Extra spaces? "Extra performance ... with a class describing ..." p 17, line 2 "describing its the" --> "describing its" p 17, lines 2-5 Could reword these sentences a bit "Each performance variable is associated with a class describing its the basic semantics. The class of a variable also denes its basic behavior, when and how an MPI implementation can change its value, and what the initial value of this variable is at the time it is either used for the first time or reset. In the following this referred to as the starting value." How about? "Each performance variable is associated with a class that describes its basic semantics, basic behavior, its starting value, and when and how an MPI implementation can changes its value. The starting value is the value the variable assumes when it is used for the first time or whenever it is reset." p 17, line 6 Change "Further, it ... to represent it." to "Further, the class of a variable ... to represent the variable." p 17, line 11 "Variables of thie class are expected to be ..." Can they be something other than an enumeration datatype? If not, I would drop the "expected to be" phrase in this sentence. If so, I would list what other types they may be. p 17, lines 10-45 Change the pattern of "The default starting value is ... (at the time the starting value is set) of the resource." to "The default starting value is ... of the resource at the time the starting value is set." p 17, line 10 - p 18, line 27 Change the pattern of "The default starting value" to just "The starting value" The term "starting value" is well defined in the intro, and adding "default" can be confusing. p 17, line 26 Change "It will be returned as an MPI_T_DOUBLE_DATATYPE" to "It is represented by an MPI_T_DOUBLE datatype." p 18, line 1 Drop "during the execution time of an application". Perhaps, I'm wrong, but I don't think this phrase is necessary. It can be misinterpretted to mean over the duration of the application, which can be wrong since the variable may count events only through some window of execution. p 18, line 3 Do you really need this clause? "by one for each specific event that is observed". If not, I'd suggest dropping it. p 18, line 24 "variables if this class" --> "variables of this class" p 18, line 26 Why only specify the units for double? How does one determine the units for MPI_T_INT and MPI_T_LONG_LONG? p 18 , line 35 Change "of an MPI application when new variables become available through dynamic loading." to "of an MPI application when new variables become available (e.g., through dynamic loading)." A similar statement is made for control variables on p 13, line 11. p 18, line 35 Extra space? "an MPI application when" p 19, line 36 Change "The argument verbosity returns the verbosity level (see Section 1.3.1) assigned by the MPI implementation to the variable." to "The argument verbosity returns the verbosity level of the variable (see Section 1.3.1)." p 19, line 38 Can it be anything other that what's in the list? "in the parameter var_class and can be" If not, I'd make this statement stronger, also break into two sentences. "in the parameter var_class. The class value must be" p 14, line 10 Same as above p 19, line 40 Change "The argument datatype returns the MPI_T datatype in which the value for this performance variable is returned." to "The argument datatype returns the MPI_T datatype that is used to represent the performance variable." p 14, line 12 Same as above p 19, line 42 "control" --> "performance" p 20, line 13 Change "Upon return, the argument readonly will be set to zero if the variable can be written or reset by the user, or one if the variable can only be read." to "Upon return, the argument readonly is set to zero if the variable can be written or reset by the user. It is set to one if the variable can only be read." Maybe change "zero" and "one" to "true" and "false", which is how flags in other MPI functions are described? p 20, line 25-28 Break this into two senences like above. And again, change "will be set" to "is set". p 20, line 27 "automatically active" --> "always active" or "continuously active" p 20, line 27 "can not by" --> "cannot be" p 21, line 6 "existing section, i.e., calls to ... of the freed session" --> "existing session. Calls to ... of a session after it is freed." p 21, line 14 Change "The type of MPI object is returned by a previous call to MPI_T_PVAR_GET_INFO in the bind argument." to "The type of MPI object a variable may be bound to is determined by calling MPI_T_PVAR_GET_INFO." p 21, line 29-36 Apply changes from earlier email about control vars. p 22, line 5 Change "Performance variables that have the continuous flag set during the query operation are continuously operating once a handle has been allocated and can be queried any time. They cannot be stopped or paused by the user." to "Performance variables that have the continuous flag set during the query operation are continuously operating once a handle has been allocated. Such variables may be queried at any time, but they may not be stopped or paused by the user." p 22, line 6 Change "All other variables are in a stopped state after their handle has been allocated; their values are not updated as the program executes, and must be started by the user." to "All other variables are in a stopped state after their handle has been allocated; their values are not updated until they have been started by the user." p 22, line 25 Change "are ignored when used with MPI_T_PVAR_ALL_HANDLES." to "are ignored when MPI_T_PVAR_ALL_HANDLES is specified." p 22, line 40 Same as above p 23, line 13 "size and fits" --> "size to hold" as described in previous email p 23, line 15 and line 35 These statements feel more like rationale. You could simply say: "The constant MPI_T_PVAR_ALL_HANDLES cannot be used as an argument for MPI_T_PVAR_WRITE." p 23, line 46 "sets of the" --> "sets the" p 23, line 47 Extra space? " to its starting" p 24, line 4 Extra space? "Readonly variables are" p 24, line 4 Change "are ignored when used with MPI_T_PVAR_ALL_HANDLES." to "are ignored when MPI_T_PVAR_ALL_HANDLES is specified." p 24, line 18 Same rationale feeling text here. p 24, line 18 Since a user must just call READ and RESET, what is the purpose of this function? Atomicity, convenience, or both? I think some explanation for the user would be useful here. p 24, line 26 "distinguishing" --> "distinguish" p 25, line 4 Dynamic loading again (see comment for p 18, line 35) p 27, line 9-13 Change "The functions will only write up to len elements into the respective array. If the category contains more than len variables or other categories respectively , the function returns an arbitrary subset; if it contains less than len variables or other categories respectively, all will be returned and the remaining array entries will not be modified." to "Starting from array index 0, each function writes up to len elements into the array. If the category contains more than len elements, the function returns an arbitrary subset. If the category contains less than len elements, the set of elements is returned in the first len array entries, and the remaining array entries are not modified." p 27, line 17 Table 1.3.9?? p 27, line 17-19 Change "None of the error codes returned by an MPI_T routine are fatal to the MPI process or invoke an MPI error handler." to "Errors returned by MPI_T routines are not fatal and do not invoke MPI error handlers." p 27, line 21 This seems to contradict the previous statements somewhat. If it's undefined, then an implementation *may* be fatal. So perhaps we should say that errors are not required to be fatal, or they are not fatal so long as the input parameters are valid, or we recommend that implementors avoid making them fatal ... p 27, line 26 Switch "complying" and extra "s" on "implementation" "a complying MPI implemetations" --> "a compliant MPI_T implementation" -Adam Martin Schulz wrote:
In preparation for the first reading at the March MPI forum, I have updated ticket #266, the proposal for the MPI Tool Information Interface and the restructuring of the profiling or tools chapter.
https://svn.mpi-forum.org/trac/mpi-forum-web/ticket/266
Changes compared to the document at the February meeting are marked in green, changes caused by the name change requested by the forum are marked in red.
Martin
<https://svn.mpi-forum.org/trac/mpi-forum-web/ticket/269>________________________________________________________________________ Martin Schulz, [email protected] <mailto:[email protected]>, http://people.llnl.gov/schulzm CASC @ Lawrence Livermore National Laboratory, Livermore, USA
------------------------------------------------------------------------
_______________________________________________ mpi-forum mailing list [email protected] http://lists.mpi-forum.org/mailman/listinfo.cgi/mpi-forum
participants (1)
-
Adam T. Moody