Comments from
the final DRI API draft, dated 8/27/2002 (URL http://www.data-re.org/dri-report-08272002.pdf
)
Comments are organized into document section (usually associated with
DRI function name), page# / line# format.
Legend:
Topic requires email discussion on data-reorg@data-re.org
email reflector
-
Copyright
-
Ken: ii/24: change copyright 1997-2001 ==> 1997-2002
-
Contents
-
Ken: iii: Entity index is missing
-
Preface
-
Ken: v/17,19-22, 23-25: refers to the old packaging of this document (which
was part technical report, and part API presentation). This is changing
to just API presentation
-
Ken: v/29: 2001 should change to 2002
-
Acknowledgements
-
Ken: vii/23: major contributors
-
Ken: viii/3: active participants
-
Ken: vii/6: list of institutions represented at meetings (denote those
that are voting institutions)
-
Ken: viii/13-16: refers to the old document packaging: technical report
+ API.
-
Glossary
-
Dennis: The introductory paragraph for the Glossary needs rewording, not
just the removal of the square brackets.
-
API conventions in chapter 1:
-
Dennis, Ken: In the example at line 27, remove the name argument so it
will match the function as it is currently defined.
-
Dennis: At line 46, I suggest you don't try to list all the functions that
use the return value for something other than an error, just give one example.
Otherwise it's hard to keep current (Ken: similar observation)
-
Chapter 2 intro sentence
-
Ken: 3/18 (change Aug 21 doc review meeting ==> final document review concluded
September 9 2002)
-
DRI_Init()
-
Ken: 5/34 (change parameters to arguments; make this a global document
change)
-
Dennis: If you've truly removed the entity index, then you have to rephrase
the sentence at 5/47 that says the entries therein are the ones brought
into scope by DRI_Init. How about "in the DRI Entity Index" ==> "in this
document" (if it's true)
-
Ken: 5/48 (reference DRI entity index; it should still be in the document!)
-
Ken: 6/14: add non-local to communication properties
-
DRI_is_initialized()
-
Ken: 7/30: add non-collective to communication properties
-
DRI_Globaldata_create()
-
Ken: USAGE RESTRICTIONS: isn't this redundant to the usage restrictions
stated in DRI_Reorg_create()?
-
DRI accessor functions can sometimes return a DRI object.
-
Dennis: For instance, DRI_Reorg_get_distribution(reorg, *dist). Because
the original object may have been deleted by the user (we say this is ok
on page1, line 33), the accessor must have created a new object to return.
So the user must have to destroy this object. If I've got this right, it
should be noted in the description of accessors on page 91.
-
Steve (response):
-
It's not necessarily true that the implementation "must" have created a
new object - it could return a reference to an existing object. However,
the user doesn't know which is true, and the specification should allow
both possibilities. Yes - the user should destroy the object. The implementation
must allow for this case, that the user attempts to _destroy an object
that he didn't actually create.
-
DRI_Overlap_create():
-
Ken: 13/15: change "overlap policy" to "edge overlap policy"
-
Dennis, Ken: Misspelling at 13/33: the word "constuctor" ==> "constructor"
-
Ken: globally, run spellchecker on all document files
-
Ken: 13/48: missing right parentheses after word "num_pos"
-
Ken: 14/15: DRI_EDGE_TOROIDAL: we should say that the order of the copied
num_pos elements is the same as they are ordered in the owning process'
memory (i.e., we are not storing the overlapped data elements in a "mirror
image" fashion).
-
DRI_Partition_create_*()
-
Ken: 16/17: change "single contiguous region" to say something more like
a contiguous range of data elements (refers to indices, not actual contiguous
storage)
-
Ken: 16/20-22: block means 0<= #blocks <=1; block-cyclic means 0<=
#blocks
-
Ken: 16/38: add something to API conventions Ch. 1 about how we use C language
operators in some documentation
-
<, >, and == comparisons
-
= assignment
-
% modulo
-
{ and }
-
etc.
-
DRI_Layout_create_packed():
-
Ken: 19/37: add that the table also shows mapping between local data indices
and corresponding memory buffer offset
-
Ken: 19/45: change text in parentheses to "each position in the order array
contains a relative packing order value for a global data dimension"
-
Ken: 20/5-7: add "i.e., order = {0, 1, 2, ...}"
-
Ken: 20/17: for all items in this list, remove the "..." from the "order
= "
-
Ken: 20/17: for all items in the list, present order = 0,1,2 as order =
{0,1,2} to mimic C language array representation
-
Ken: 21: table 2.1: change 2nd column header to "M(y,x) => ..." [reverse
x,y to y,x]
-
Ken: 22: table 2.2: change 2nd column header to "C(z,y,x) ==> ..." [reverse
x,y,z to z,y,x]
-
DRI_Layout_create_aligned():
-
Dennis: The LIS for rel_align says "array of integer" but in C it is a
pointer to an "unsigned" int. They should match, and I think unsigned
is what we want. Alignment values are always unsigned, and there
are no magic keywords for specifying special cases here which was another
reason why we sometimes wanted to use negative numbers.
-
Ken has similar observation
-
Ken: 23/46: associate dimsize values with the DRI_Globaldata_create dimsizes[]
array argument values
-
Dennis, Ken: Formatting problems with "such as" and "and" on the last line
of page 23
-
Dennis, Ken: Similar problems at 24/4 and 24/6
-
Dennis: In the first example starting at 24/10, I suggest switching the
position of sentence 2 with sentence 3. Sentence 3 seems more general
and should go first, and sentence 2 is starting to address alignment and
logically precedes sentence 4.
-
Ken: 24/12,14: add {} around rel_align and order array values
-
Ken: 24/40: COMMUNICATION PROPERTIES: missing the word "collective"
-
DRI_Distribution_create() [replication issue]
-
Ken: 25/19: change "data distribution" to "partitioning constraints"
-
Ken: 25/35: to sentence ending with "same as the number of dimensions attribute
...", add a note that this is the same as the ndims argument to DRI_Globaldata_create
-
Dennis: see email thread that ensued on data-reorg@data-re.org, subject
"comments on DRI_Distribution_create (replication)"
-
DRI_Dataspec_get_size()
-
Dennis: The text from 27/48 through 28/11 should be moved up into the discussion
section, and the two advice to users blocks should be combined as they
are addressing the same thing.
-
DRI_Datapart_create()
-
Ken: 31/39: change "the DRI_Datapart object" to "the returned DRI_Datapart
object"
-
Ken: 31/43-45: sentence beginning "In this use-case, ..." can be cleaned
up a bit
-
Ken: 32/16: add more space after background and next paragraph
-
Dennis: Your "DRI_changes_from_08212002_docreview" says that for Datapart_create
you changed "buffer size" to "buffer size in bytes". If this was
supposed to occur at 32/39, then it didn't happen.
-
Dennis: Communication properties: I don't believe
this should be collective. For instance, rank 0 might do calculations
for all the processes for some reason but the other ranks don't.
-
DRI_Reorg_create()
-
Ken: 37/23: "buffers" ==> "memory buffers"
-
Dennis: Starting with a really nitty nit: At 37/28 the C binding has the
parentheses immediately after the function name, then a space. In
nearly all other cases, it is the other way around.
-
Dennis: At line 33, the background section doesn't seem appropriate here
in Reorg_create like it was in Datapart_create. I would have instead
inserted it into DRI_Reorg_get_datapart and DRI_Reorg_get_datapart_nomem.
-
Ken: 38/24: insert a brief description of the name argument here (there
is none currently)
-
Dennis, Ken: At 38/32: "See the MIDDLEWARE ADAPTER section ..." is no longer
appropriate. I guess the discussion of groups is now in the COMMUNICATION
PROPERTIES section.
-
Ken: 39/7: add "Currently in DRI" to the sentence beginning "Teh only attribute
controllable ..."
-
Ken: 39/21: add space between paragraphs
-
Ken: 40/12: change to "packing order and relative byte alignment attributes
in the layout argument passed to DRI_Distribution_create"
-
DRI_Reorg_connect()
-
Ken: 41/17: extra space between "pointer" asterisk and input argument name
in C binding (fix this globally in the document)
-
Ken: 41/29: change to "non-local, collective, and
collective with ..."?
-
DRI_Reorg_get|put_datapart
-
Ken: 43/29,34: should dpo argument to put_datapart be INOUT?
-
Ken: 43/42: change "to the memory" ==> "to a memory"
-
Ken: 44/4: add "and the blockcount attribute of dpo
will be zero"?
-
Ken: 44/9: add to end of 1st sentence (instead of
sending a full buffer of data)
-
Ken: 46/11: "This buffer is sent" ==> "This computational
output buffer is sent ..."
-
Ken: 46/12: "Additionally, the input buffer is returned
..." ==> "Additionally, the computational input bufer is returned ..."
-
Ken: 46/19: "e.g., if a DRI_Reorg_Datapart object
is ..." ==> DRI_Datapart object
-
Ken: 48/6: "in DRI_Reorg_create" ==> "in the DRI_Reorg_create
documentation"
-
DRI_Reorg_tryget_datapart()
-
Dennis, Ken: 49/44: "When DRI_SUCCESS is returned" ==> "When DRI_SUCCESS
or DRI_NOTIFY_VALUE is returned"
-
Dennis: 49/44: remove the word "both"
-
Dennis, Ken: 49/36-40: The description of the notify pointer result value
("points to" the value) and of the dpo object (specifically that use of
it will return an error) should all be the same as that used for get_datapart
on 44/14-18.
-
Dennis,Ken: Your change notes for DRI_Reorg_tryget_datapart say that the
ptr and dpo arguments will be unchanged if it returns DRI_BUFS_NONE.
That change didn't happen. Change "undefined" to "unchanged" at 49/31.
-
Dennis: And add a period to the end of the sentence at 49/28.
-
Ken: 49/35: add to end of sentence "(instead of a full buffer of data)"
-
DRI_Reorg_notify()
-
Ken: 51/25: add to end of 1st sentence "(instead of sending a full buffer
of data with DRI_Reorg_put_datapart)"
-
Dennis: At 51/28 change "sets the output ptr argument to the 32-bit message"
to "sets the output ptr argument to point to the 32-bit message"
-
DRI_Reorg_process_inplace()
-
Ken: 53/20: C binding requires additional pointers: DRI_Reorg *side_in,
DRI_Reorg *side_out
-
Dennis, Ken: 53/16: Change "to occurs" ==> "that occurs"
-
Dennis, Ken: 53/28: Change "num_buffers >= 0" to "num_buffers > 0" as we
don't allow no buffers at all!
-
Ken: 53/32: the application can also call DRI_Reorg_tryget,
right?
-
Dennis, Ken: 53/41: When you added the ptr argument to the get_datapart
call, you also mistakenly added it to the put_datapart call. Remove
the ptr argument from put_datapart(...).
-
Ken: 53/42: remove "Users should" from beginning of sentence.
-
DRI_Reorg_get_bufstate()
-
Dennis: 55/27: change "DRI_Reorg_bufstatus" to "DRI_Reorg_bufstate"
-
Dennis, Ken: 55/32-33: delete the last line of code
-
Dennis, Ken: 55/35: add a comma after "DRI_Reorg_get_num_buffers"
-
Ken: 55/38: also applies to DRI_Reorg_tryget_datapart,
right?
-
DRI_Reorg_get_datapart_nomem()
-
Dennis: 57/29: Delete this sentence as it's not clear which Reorg_create
you can call it after. And anyway you will have to have a reorg object
to call it.
-
Dennis: 57/43: I believe the accessor DRI_Datapart_get_dataspec should
exist because you may want to get the dataspec from a dpo when you don't
have the corresponding reorg object around to query. So add DRI_Datapart_get_dataspec
to the list here, and also to the accessor pages (page 90).
-
Dennis: 58/6: Delete the word "that".
-
Ken: 57/46: wondering why we cannot destroy the returned
datapart object with DRI_Datapart_destroy
-
Ken: 58/8. Add another sentence saying that the returned
dpo object cannot be passed as input to DRI_Reorg_put_datapart, or DRI_Reorg_notify.
-
DRI_Datapart_get_blockcount()
-
Dennis, Ken: 61/26: "greater than or equal to 1" -- According
to 63/32, block cyclic can result in 0 blocks (you just changed this on
page 63). So this page should match: "greater than or equal to 0"
-
DRI_Datapart_get_blockinfo()
-
Ken: 63/16: change (identifier) to number
-
Ken: 63/18: change arg description to "associated with the specified data
distribution block"
-
Ken: 63/21: change "unsigned integer" to "unsigned int" in C binding
-
Ken: 63/25-30: what are the contents of blockinfo
output argument when DRI_DATAPART_NO_BLOCKS is returned?
-
Dennis, Ken: At 63/28, I was initially a bit confused about how the value
was returned. I suggest changing the wording from "then the constant
DRI_DATAPART_NO_BLOCKS is returned to the caller." to "then the function
returns DRI_DATAPART_NO_BLOCKS.
-
Ken: 63/38: say that number of data dimensions is accessed with DRI_Blockinfo_ndims()
-
Dennis: Also, a missing period after the right parentheses at 64/23.
-
Ken: 64/39: no need to number the example; there is only one
-
Ken: 64/47: "mod 1" ==> "mod = 1" (add = sign)
-
Ken: 64/48: "on the global dataset edges" ==> "... edge" (not plural, since
referring to left overlap)
-
Ken: 65/3: "on the global dataset edges" ==> "... edge" (not plural, since
referring to right overlap)
-
Ken: 67/7: add that the DRI_Datapart object must be under application control
when this call is made, else an error is returned.
-
DRI_Blockinfo_length
-
Ken: 77/22: "consecutive data" ==> "consecutive global data"
-
Ken: 77/26: "... calling process" ==> "... calling process in dimension
dim"
-
DRI_Blockinfo_stride
-
Ken: 79:23-26: add that the value returned is always >=1
-
DRI_Blockinfo_left|right_
-
Dennis, Ken: These two comments apply to both DRI_Blockinfo_left and _right
pages:
-
In the C bindings, remove the "*" from the blockinfo argument for all four
functions.
-
In the descriptions of the other Blockinfo accessors, there is a final
sentence of the description saying "If the blockinfo argument is not valid...".
That sentence is missing for these four functions.
-
general comment about Language Independent Specification (LIS)
-
Dennis: The LIS convention is to use "integer" vs "int" for argument types.
I noticed two places where "int" is used and should be changed. You
might scan for others I missed.
-
Dennis: 31/16 and 31/18 (arguments for DRI_Datapart_create)
-
Dennis: 35/17 (argument for DRI_Datapart_get_buffersize)
-
DRI accessor functions
-
Dennis, Ken: 85/16: add DRI_Datapart_get_dataspec
-
Dennis, Ken: 86/10: add DRI_Partition_get_blksz
-
Dennis: 87/19: add LIS for DRI_Datapart_get_dataspec
-
Dennis, Ken: 88/36: The convention here for array arguments is to provide
the dimension you want and get just one entry back. The didn't happen
for DRI_Distribution_get_parts. Add a second argument "dim" and return
just a single DRI_Partition. Change "parts" to "part" for the argument
name. Make the function name singular here and at 85/39 and 90/42.
-
Dennis: 88/41, Ken: add LIS for DRI_Distribution_get_layout
-
Dennis: 89/14: Depending on the resolution of an inconsistency with a DRI_Layout_create
argument (is rel_align signed or unsigned -- the LIS and C disagree), this
may be an "unsigned integer".
-
Dennis: 89/42: Add LIS for DRI_Partition_get_blksz
-
Dennis, Ken: 90/15: Make the function name DRI_Globaldata_get_dimsizes
singular to match other functions that return just one entry of an array.
Change it also at 91/13 (it's already correct at 86/18).
-
Dennis, Ken: 90/24: Typo -- remove "DRI_Distribution *"
-
Dennis: 90/26: insert C binding for DRI_Datapart_get_dataspec
-
Dennis, Ken: 90/29: insert C binding for DRI_Reorg_release_name
-
Dennis, Ken: 90/42: As noted at 88/36 above, singularize the function name
and "part" argument, and add a new argument "dim". Remove one of
the "*" for the part argument.
-
Dennis: 91/7: insert C binding for DRI_Partition_get_blksz
-
Dennis, Ken: 91/13: singularize the function name DRI_Globaldata_get_dimsize
-
Dennis: 91/27: Add to the discussion that the user must destroy the DRI
objects that are returned by these accessor function. Possibly, we
should itemize the exact cases to avoid confusion:
-
DRI_Datapart_get_distribution
-
DRI_Reorg_get_distribution
-
DRI_Distribution_get_globaldata
-
DRI_Distribution_get_part
-
DRI_Distribution_get_layout
-
DRI_Partition_get_left_overlap
-
DRI_Partition_get_right_overlap
-
DRI_Destroy functions
-
Dennis, Ken: We've discussed this elsewhere, but all arguments should have
* according to 1/24.
-
Dennis: In Layout destroy, the COMMUNICATION PROPERTIES text is missing:
local, non-collective.
-
DRI_Datapart_destroy
-
Ken: 105/27: missing DRI_Reorg_tryget_datapart? Equivalent
to say that you can only destroy those created by DRI_Datapart_create?
-
Chapter 3 comments
-
Dennis: Change "DRI-1.0" to "DRI".
-
Dennis: Because these aren't just error codes, reword the second sentence
to The C bindings have the same format as the LIS codes." or "The codes
for the C binding are the same as the LIS codes.".
-
Dennis: I think you ought to add something along the lines of "Other implementation-specific
codes may be returned as described in Chapter 1."
-
DRI_Blockinfo accessors that take a dimension number "dim" argument:
-
Ken: I believe the description of the dim parameter is partially correct,
and partially incorrect. It says:
-
The dim parameter can be interpreted as an index into the order parameter
(passed to a DRI_Layout constructor).
-
The programmer can always pass dim=0 to DRI_Blockinfo_length (and similar
per-dimension accessors) to refer to the most contiguously packed dimension
in memory
-
Ken: I believe that the first item is wrong. I think that the dim parameter
value is equal to the value of order[gdodim]. So, if you want to know the
gdodim that you are processing with, you need to do a reverse lookup, searching
for the value dim in the order[] array. The position at which you find
the value is the gdo dimension number.
-
Dennis (response):
-
You are correct. Your item 2. is the behaviour we want and what is
described in the rationale, so the functions are working correctly.
How you are going to reword the dim description in 25 words or less should
be interesting!
-
Dim is an index into a virtual array for this distribution that doesn't
ever appear anywhere. And as you point out, to figure out the global
dimension that corresponds with each dim here, you have to start with the
datapart that the blockinfo came from, get its distribution, get the distribution's
layout, from the layout get each global dimension's order value, and then
build yourself a reverse table.
-
Since the blockinfo is supposed to have "everything" conveniently available,
it would be appropriate to add a new blockinfo per-dimension entry for
global dimension -- and, of course, the new function to retrieve it.
-
DRI_Blockinfo() accessors, Rationale paragraph
-
Dennis: In all the blockinfo accessor rationale paragraphs, there is an
extra space before a comma on the third line of the paragrph. (e.g., 77/36)
-
Chapter 3 return codes
-
Ken: 109/16: DRI_ERR_some-error is not the format. Remove this
-
Annex A Journal of Development
-
Ken: in general, say that the features not specified in DRI-1.0 may be
implemented in non-portable ways in the meantime
-
Ken: add the following topics to journal of development
-
split data types (e.g., complex numbers, RGB, etc.)
-
stripmining (piecemeal data production/consumption on a smaller granularity
than the global data object)
-
interoperation with VSIPL
-
Non-CPU endpoints (e.g., devices)
-
Annex B Cornerturn Example Code
-
Ken: use DRI_Reorg_process_inplace to shorten the example even more
-
Bibliography