Comments on SRFI 122 draft 14 Sudarshan S Chawathe (04 Dec 2016 19:21 UTC)
Re: Comments on SRFI 122 draft 14 John Cowan (04 Dec 2016 23:13 UTC)
Re: Comments on SRFI 122 draft 14 Bradley Lucier (15 Dec 2016 18:46 UTC)

Re: Comments on SRFI 122 draft 14 Bradley Lucier 15 Dec 2016 18:46 UTC

I just updated my github repo and sent a pull request to srfi-122.

On 12/04/2016 02:21 PM, Sudarshan S Chawathe wrote:
> These questions and comments refer to draft 14 of SRFI 122 and are in
> roughly document order (not significance order).
>
>   * (trivial) Extra ';;;' at the end of the description of
>     interval-upper-bounds->list.

Fixed.

>
>   * Should interval= be named interval=? instead, by analogy with
>     string=?, char=?, etc.?

I decided the ? looked redundant.

>
>   * Since interval-intersect? returns a usable 'true' value, should it
>     be named interval-intersect, by analogy with member, etc.

Fixed.

>
>   * In the description of storage-class-default et al., the invocation
>     of make-storage-class has the positions of getter and setter
>     switched.

Fixed.

>
>   * Also at the above location, storage-class-length is missing from
>     the sentence describing the return values.

Fixed.

>
>   * Arrays and intervals are specified as being Scheme types different
>     from all others.  However, storage classes do not seem to have
>     this requirement.  I am guessing this design is intentional,
>     perhaps to make some optimized implementations easier, but am not
>     sure.

Made distinct.

>
>   * (nit) In the description of procedure array?, I assume the
>     intention is (the obvious interpretation) that the returned value
>     must be #f in other cases but, strictly speaking, that is not
>     required by the current phrasing.  Ditto for mutable-array?.

Fixed.

>
>   * For completeness, perhaps the description of array-domain and
>     array-getter should allow an optional setter in the invocation
>     used in the "if" part of the first sentence there.

Fixed.

>
>   * In the description of specialized-array-default-safe?, is the #t
>     intentionally specific, or is any true value (i.e., anything other
>     than #f and #false) permissible there?  It would be useful to
>     clarify in any case.  There are a few other similar occurrences in
>     the document (true v. #t).  Similarly, there are a few places
>     where an argument is restricted to being a boolean.  I assume this
>     would be interpreted as limiting valid values to those for which
>     boolean? yields true.  Unless this restriction is intentional, it
>     may be better to go with the usual Scheme convention of
>     interpreting any values as boolean.

I generally restrict arguments to booleans, it allows to check for
errors earlier and give more accurate error reports.

>
>   * The position of the "new value" argument for array setters (first)
>     is different from that used by the storage-class setters.  I
>     assume this decision is motivated by reasons similar to those
>     noted in SRFI 63, but highlighting of the difference may be
>     useful.

I prefer to let this pass without comment.  Tell me if I'm wrong.

>
>   * (noted in an earlier by John Cowan) specialized-array may be
>     better named make-specialized-array.  (And perhaps ditto for the
>     u16-array example procedure later.)

Fixed.

>
>   * In the description of specialized-array, there's a missing closing
>     parenthesis near the end of the sentence just before "If safe is
>     #t...".

Fixed.

>
>   * Above location: Though implied, the sentence "If safe is #t..."
>     should perhaps say something like "in addition to the following".

Fixed.

>
>   * In the description of specialized-array, the code defining
>     (array-setter array) has storage-class-getter where it should use
>     storage-class-setter.

Fixed.

>
>   * I am confused by the description of array-indexer and array-body.
>     Regarding the statement "(array-indexer array) is assumed to...":
>     Isn't (array-indexer array) provided by the implementation instead
>     of the user (directly)?  If so, then the assumption seems like it
>     should be a guarantee by the SRFI/implementation to the user.
>     Also, I could not find a description of array-body (although I can
>     guess, especially given the sample implementation, that it returns
>     the implied backing-store object for the storage class used by the
>     specialized array).

Originally, I thought there would be a really low-level way to specify a
specialized array where the user specified the body and the indexer,
etc.  This language is left over from that "hopeful" time.  (Some of the
is noted as the last item in "Issues and Notes".)

I think I'd like to leave language that might apply to a future
low-level interface.

>
>   * There are three instances of "specialized-array-indexer" in the
>     document that I assume refer to the "array-indexer" procedure
>     above, so that one name should be changed to the other.

Fixed.

> (Perhaps
>     it would be useful for the procedures that can only be applied to
>     specialized array to have a 'specialized-' prefix or some other
>     distinguishing feature in their name.)

I appreciate the sentiment, but I'm going to go with the shorter names.
>
>   * (minor) The example on "shearing" seems to have extra indentation
>     for the (define b ...) portion.

Fixed.

>
>   * (minor) In the description of array-permute, 'Array' should be 'array'.

Fixed (and also in array-translate, from which that sentence was copied).

>
> 	* (minor) The explanation of array-reverse may be easier to
>     understand (and a bit shorter) if the common parts of the code
>     were factored out.  Perhaps something along the lines:
>
>       (define (flip-multi-index array flip? . multi-index)
>         (map (lambda (i_k flip?_k l_k u_k)
>                (if flip?
>                    (- (+ l_k u_k -1) i_k)
>                    i_k))
>              multi-index
>              (vector->list flip?)
>              (interval-lower-bounds->list (array-domain array))
>              (interval-upper-bounds->list (array-domain array))))

I took your advice.  (That's how the implementation of many of these
functions is structured in any case.)

>
> 	* Would it be desirable for array->list to guarantee lexicographic
>     traversal of the array (similar to the guarantee provided by
>     array->specialized-array)?  If such a guarantee is added, then it
>     would also propagate to array-fold and array-fold-right given
>     their definitions in terms of array->list.

It already says "in lexicographical order".  Need I add anything more?

>
>   * The description of array-map (and array-for-each) refers to the f
>     argument as 'function' instead of the more conventional
>     'procedure'.  Since 'function' is used in its mathematical sense
>     elsewhere in the document, it may be better to change these
>     instances to 'procedure'.

Fixed.

>
>   * Given array-every?, should array-any? also be provided?

I got rid of array-every? (see below).

>
> 	* By analogy with SRFI-1, should array-every? (and array-any? if
>     added) return the true value (if any) from the final proc
>     invocation?  (In that case, the names array-every and array-any
>     would be more conventional.)
>
>   * Should array-every? (and array-any? if added) provide any
>     tail-call guarantees similar to the analogous procedures in SRFI 1
>     (or would that create implementation problems)?

I wrote, documented, and tested array-every and array-any to
more-or-less conform to SRFI-1's every and any.

>
>   * SRFI 25's array-ref and array-set! permit array indices to be
>     provided either as separate arguments or as a vector or as a
>     1-dimensional array.  Would a similar strategy be useful for the
>     relevant procedures of this SRFI?

I don't want to do this.  Partly this SRFI is a meditation on how to
manipulate scheme "values" in the special case of intervals.

>
>   * (very minor) Perhaps a small change to the PGM example would
>     showcase the SRFI features a bit more: It could use a more
>     restrictive storage class for the array it builds, based on the
>     PGM header (i.e., u8-storage-class for greys < 256,
>     u16-storage-class otherwise, etc.).  But I may be missing some PGM
>     format subtleties here.

I added a line.

Thanks.

Brad