Re: review/merge request: wip-array-refactor

[Neil Jerram]

Andy Wingo <address@hidden> writes:

> Hi Neil,
>
> Thanks for the review.
>
> On Wed 22 Jul 2009 23:48, Neil Jerram <address@hidden> writes:
>
>> I have two overall questions in mind.
>>
>> - What do you have in mind as regards releasing this?  Even though it
>>   looks good, I think it would be better to let it mature for a while,
>>   and hence not to put it into 1.9.x/2.0.  (And we're not short of new
>>   stuff in 1.9.x/2.0!)
>
> Personally I would prefer that it come out in 2.0. I'm fairly (but not
> entirely) confident of its consistency as it is, and quite confident
> that it is a more maintainable, and hence fixable, codebase.

I could be wrong, but I don't intuitively feel comfortable with that.
It just feels too quick/early.

On the other hand, I think this is really valuable work, and
absolutely don't want an interval of years or months before it gets
out there.

What is our release plan after 2.0?  I don't know.  I'd like something
more dynamic than the very long intervals between major releases that
we've had in the past.  But it seems there is a conflict between

- major releases being the points at which we can break the API/ABI
  (with accompanying documentation)

- wanting to have such releases more frequently than in the past, so
  that good new stuff gets out quicker

- wanting not to create grief for Guile users by changing the API/ABI
  frequently.

Is there a solution?

> The reason that I want it in is that bytevectors are a nice api for I/O,
> but once you have data in memory, often it's best to treat it as a
> uniform array -- be it of u8 color components, or s32 audio samples.
>
> Uniform vectors are almost by nature "in flight" between two places.

(Not sure I agree.  I'd say uniform vectors are mostly holding numbers
in a computation, or for plotting on a graph.)

> But
> it is the bytevector that is most equipped to "fly", via e.g. (rnrs io
> ports); but bytevectors do not provide an ideal API for manipulation.
> s32vector-ref is a better API than bytevector-s32-native-ref, if only
> that the offset of the former is in native units (s32 elements), and of
> the second in bytes.
>
> I guess what I wanted to do was break uniform vectors out of their type
> ghettoes. Like Perlis said, "Pascal is for building pyramids --
> imposing, breathtaking, static structures built by armies pushing heavy
> blocks into place. Lisp is for building organisms -- imposing,
> breathtaking, dynamic structures built by squads fitting fluctuating
> myriads of simpler organisms into place." I should be able to put a
> 64-bit float into two 32-bit words, when I'm writing an instruction
> sequence for a 32-bit machine, without having to change my data type --
> they are just bytes anyway.

That sounds to me like motivation for adding a richer API to
bytevectors (and which could all be in Scheme), not necessarily for
the deep unification of uniform and byte vectors that you have coded.

TBH, with your refactoring up to this point, I still don't have the
overall picture (arrays, uniform vectors, bitvectors, strings etc)
firmly in my head.  I'd like to do that and then reconsider your
points above.

>> - What about the header file incompatibilities?  I.e. the problem that
>>   if there are applications out that that #include <libguile/ramap.h>,
>>   or another of the renamed ones, they will stop working.  Do you
>>   think we are OK just to document this well, or should we do more
>>   than that?
>
> I thought that our only support API was that provided by #include
> <libguile.h>.

Yeah, actually (and given that Ludovic has said the same) I'm happy
with that too.  Sorry for raising unnecessary concerns!

>>>   (u8vector-ref #u32(#xffffffff) 0) => 255

Note that using #xffffffff here glosses over the endianness problem.

(I think my inclination at this point is that I'd prefer explicit
conversions.)

> I ought to be able to get at the bits of a packed (uniform) vector. The
> whole point of being a uniform vector is to specify a certain bit layout
> of your data.

Huh?  I would say it is to be able to store numbers with a given range
(or precision) efficiently, and to be able to access them efficiently
from both Scheme and C.

>>>   (u8vector? #u32(#xffffffff)) => #f
>
> However, we need to preserve type dispatch:
>
>   (cond
>     ((u8vector? x) ...)
>     ((s8vector? x) ...)
>     ...)
>
>>>   (bytevector? #u32(#xffffffff)) => #t
>
> This to me is like allowing (integer? 1) and (integer? 1.0); in
> /essence/ a #u32 is an array of bytes, interpreted in a certain way.

I think you have in mind that all uniform vectors are filled by
reading from a port, or are destined for writing out to a port.

That is an important use, but there is another one: preparing
numerical data for handling in both C and Scheme.  In this use, the
concept of an underlying array of bytes plays no part.

>>> commit 86d88a223c64276e7cd9b4503e7e2ecca5aae320
>>> Author: Andy Wingo <address@hidden>
>>> Date:   Thu Jul 16 21:51:47 2009 +0200
>>>
>>>     remove deprecated functions from unif.c
>>>     
>>>     * libguile/unif.h:
>>>     * libguile/unif.c: Remove deprecated functions.
>>>     
>>>     * module/ice-9/deprecated.scm: Remove array-related deprecated
>>>       functions.
>>>     
>>>     * NEWS: Update.
>>
>> I can understand why a lot of the NEWS deltas evaporate when they are
>> logically merged into the 1.8-2.0 section, but I'm not sure about
>>
>> - the entry for Brainfuck
>>
>> - the entry on scm_set_port_seek and scm_set_port_truncate
>>
>> Aren't these entries still of interest?
>>
>> The rest of this commit looks fine.
>
> Yes they are. Good catch.

Cool, thanks.

> I'll finish this response, and the response to Ludovic, later -- but I
> wanted to get out the rationale now. Thanks again for the review!

No problem, it was a good read!

Regards,
        Neil