Wolfgang Corcoran-Mathe <xxxxxx@sigwinch.xyz> writes:

> Thanks to Vladimir for putting this SRFI together.  Combing through
> all of SICP's forms for the primitive, unspecified ones must have
> been a lot of work, and this SRFI seems to have determined the
> essentials.  I have a few short comments.
>
> (1) The comments in the examples for `runtime' and `random' suggest
> that these procedures print values, which doesn't seem to be intended.

Fixed.

> The `runtime' example also shows the procedure returning a
> floating-point value, whereas it is specified to return an integer.

Fixed.

> (2) Does `runtime' need to return the "system uptime"?

It is unclear from the book itself. System in SICP is usually used as a
"Scheme system", so that could mean "how long the interpreter has been running",
which is hard to determine. On the other hand, it could also mean "how
much the operating system has been running", that is an equivalent of
"uptime" UNIX command.

On the other hand, on MIT/GNU Scheme it is not even that, it returns
"the number of seconds since the last REPL prompt".

I decided that it is confusing enough to settle with returning the
number of microseconds since the Epoch.

>If it
> provides a different integer seed on successive calls, isn't that
> enough for this SRFI's purposes?

It's not about seeding the random number generator. It is about
measuring code performance, so it has to monotonically increase with
time.

> (3) In the example for parallel-execute, read "May assign to x any
> of the following" for "May assign x to any of the following".

Fixed.

> (4) Is there a rationale for recommending that stream-null? and
> the-empty-stream be defined as null? and (), respectively?

Well... MIT/GNU-Scheme does it like this.

> (5) Since the stream implementation is contrasted with that of
> SRFI 41, it might be worth mentioning that 41's (even) stream-cons
> will *not* work as an implementation of cons-stream.

Added.

> (6) (very minor) The formatting of the streams section is cramped
> and probably could be improved with some paragraph tags.

A good point... I don not know how to solve it. I think it is not
allowed by HTML to insert p-tags into dl-tags. Maybe span should be
used... or some clever CSS. Pull requests welcome, I don't know how to
solve it.

> (7) Whether this test passes is non-deterministic based on the
> specification of `runtime':
>
>     (check (> (- (runtime) (runtime)) 0) => #t)
>
> It failed during my CHICKEN test run, which is unsurprising, given
> that the sample implementation uses current-second.  The comparison
> here is made even more questionable if `runtime' returns an inexact
> integer (as does the sample implementation).

Excellent point. Sorry, I forgot to update the sample implementation,
now it is implemented like this:

```
(define (runtime) ;; r7rs
   (round (* (current-jiffy) (jiffies-per-second) #e1e6)))
```

> (8) The sample implementation needs to import (scheme time) for
> current-second.

And for `current-jiffy`. Fixed.

> (9) The (scheme small) subset imported by the sample implementation
> seems to chibi-scheme specific.  Perhaps it should be replaced with
> (scheme base) for portability.

Fixed.

> With the exception of the non-deterministic `runtime' test mentioned
> above, all the included tests pass on CHICKEN+r7rs.

If I can bother you to test the `current=jiffy`-based one once again,
please?

> Best regards,

That's an excellent review! Thank you very much!

I think I will be announcing the last call after this review.

--
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)