Review of SRFI 122 draft #8 John Cowan (23 Aug 2016 20:50 UTC)
Re: Review of SRFI 122 draft #8 Bradley Lucier (24 Aug 2016 21:40 UTC)
Re: Review of SRFI 122 draft #8 John Cowan (24 Aug 2016 22:43 UTC)
Re: Review of SRFI 122 draft #8 Bradley Lucier (25 Aug 2016 23:07 UTC)
Re: Review of SRFI 122 draft #8 John Cowan (26 Aug 2016 01:51 UTC)
Re: Review of SRFI 122 draft #8 Bradley Lucier (26 Aug 2016 14:49 UTC)

Re: Review of SRFI 122 draft #8 Bradley Lucier 24 Aug 2016 21:40 UTC

On 08/23/2016 04:50 PM, John Cowan wrote:
> I did a full review of draft #7, but before I could post it, draft #8
> came out, which is much easier to understand,

Which is some progress, I suppose.

I'll make comments on some things, and will think about others.

> 1) In the "Other Examples" section, the lengthy outputs should be folded
> by hand across reasonable line boundaries.  When I try to print the SRFI
> using Chrome, it forces everything to be displayed by using a small and
> hard-to-read font size.  I ended up removing the section before
> printing, which means I have not reviewed it.

I made some changes to an internal draft at

https://github.com/gambiteer/srfi-122/

I haven't generated a pull request yet for the main repository.

>
> 2) I think the default value should be provided when a specialized array
> is created, not when a storage class is created.  In addition, the maker
> function of a storage class should be callable with a single argument,
> in which case the function may return a body full of uninitialized
> garbage.  This is not useful for generalized arrays, but may save time
> for specialized arrays whose initial values are uninteresting.

I agree that it may save some time, but I believe that returning
uninitialized garbage is generally frowned upon for various reasons,
including security reasons.

>
> 3) The SRFI should say that generic-storage-class behaves *as if* the
> specified definition were used.  This allows faster versions of
> vector-ref, vector-set!, etc. to be used.

Fixed in internal draft.

>
> 4) In the following paragraph, "generic-storage-class" is not in code font.

Fixed in internal draft.

> 5) It should be explicitly stated that arrays, specialized arrays, and
> intervals are disjoint types.

I'm not going to argue against this, but let me ask "why?"  Translations
and permutations are not disjoint types.  And a specialized-array is an
array, so in what way are these types disjoint?

> 6) The section on specialized arrays is labeled "Global Variable"
> instead of "Specialized Arrays".

Fixed in internal draft.

> 7) It is implied that specialized-array-default-safe? can be mutated.
> Neither R6RS nor R7RS permits the mutation of exported variables.  A
> SRFI 39 parameter would work, or you could just remove this
> functionality and require people to pass #t to `specialized-array` if
> they want a (doubly) safe array.

I introduced two procedures, specialized-array-default-safe? and
specialized-array-default-safe?-set!

>
> 8) You should make clear what the effects are of mutating the vectors
> passed to `interval` and returned by `interval-upper-bounds` and
> `interval-lower-bounds`. I recommend that it be an error, which means
> none of these operations needs to copy the vectors.

In the reference implementation there is no effect on mutating the
arguments to interval or the results of interval-upper-bounds->vector or
interval-lower-bounds->vector.  I prefer to keep it this way.

So, do you think I still need to explain something?

>
> 9) The definitions of getters and setters are backward compatible with
> Common Lisp and other SRFIs, but make for implementation
> inefficiencies.  When a multi-index exists only as a sequence of
> arguments to a procedure, Scheme requires that the implementation of
> procedure calls reify the arguments as a list before being passed to the
> called procedure,

I don't know what you mean.  The (heavily-macrofied) C code generated by
Gambit for (setter 1 0 0) in

(define (test)
   (let ((getter (array-getter A))
	(setter (array-setter A)))
     (setter 1 0 0)
     (getter 0 0)))

is

    ___SET_R3(___FIX(0L))
    ___SET_R2(___FIX(0L))
    ___SET_STK(-5,___R1)
    ___SET_R1(___FIX(1L))
    ___SET_R0(___LBL(4))
    ___JUMPGENNOTSAFE(___SET_NARGS(3),___STK(-5))

The three arguments are stored in registers R1, R2, and R3, the return
address is stored in R0, and there's a direct jump to the label for the
code for getter, which has been put into ___STK(-5).  I don't see any lists.

> and this has to be done *every* time the procedure is
> called.
>
> If you instead require the caller to reify it as a vector and pass the
> vector, it can be reified just once and mutated on successive calls.
> Admittedly, this is somewhat less convenient, but if you want
> convenience more than performance, (getter (vector 1 2 3)) is only
> slightly more verbose than (getter 1 2 3), and may indeed allocate less
> storage.
>
> Using this approach for setters allows the new value to be the last
> argument rather than the first, in conformity with setters for other
> data types.  It likewise means that the mapping procedure passed to
> `specialized-array-share` simply accepts a vector and returns a vector,
> rather than messing with variable arguments and multiple values.
>

> 11) Here are some procedures you might want to consider adding:
>
> Constructor: array-tabulate (populates the array with successive calls
> to a tabulation function with argument 0, 1, 2, ...)

One could do

(define (array-tabulate f domain #!optional (storage-class
generic-storage-class) (safe? (specialized-array-default-safe?))
   (array->specialized-array
    (make-array domain
	       (let ((i 0))
		 (lambda args
		   (let ((result (f i)))
		     (set! i (+ i 1))
		     result))))
    storage-class
    safe?))

array->specialized-array does a lot of error checking, I'd like to avoid
repeating that code in other procedures.

>
> Array-ref and array-set!  convenience methods for getting and setting
> without explicitly retrieving the getter/setter.

I'm trying to offer alternatives to word-at-a-time array processing in
this SRFI.

>
> Whole-array functions:  array-fold (fold in lexicographic order),
> array-reduce, array-scan (APL), array-count, array-index.  All of these
> are basically equivalent to converting the array to a list and operating
> on the list, but much more space and time efficient.

I don't know what these do: array-scan, array-count, array-index.

I have array-reduce; how does it differ from array-fold?

>
> Additional simple transformation on all arrays:  array-diagonal (returns
> a 1d array containing the diagonal elements up to the smallest dimension)
>
> Others:  array-append (along a specified dimension), array-repeat
> (append to itself N times along a specified dimension),
> nested-list->specialized-array (needs number of dimensions),
> nested-vector->specialized-array, array->nested-list,
> array->nested-vector, inner product (two procs), outer product (one proc).

Perhaps these are best to leave to another SRFI.  (I have used
array-append in my own codes.)