digego / extempore

A cyber-physical programming environment
1.41k stars 127 forks source link

Callback (Scheme) misbehaves with non-top-level closures #377

Closed adql closed 4 years ago

adql commented 4 years ago

I was working through the note-level-music guide and reached this one:

(define play-seq
  (lambda (time plst rlst)
    (play-note time synth (car plst) 80 (car rlst))
    (callback (+ time (* .5 (car rlst))) 'play-seq (+ time (car rlst))
              (if (null? (cdr plst))
                  '(60 62 65 69 67)
                  (cdr plst))
              (if (null? (cdr rlst))
                  '(11025 11025 22050 11025)
                  (cdr rlst)))))

(play-seq (now) '(60 62 65 69 67) '(11025 11025 22050 11025))

which works fine. The text suggests as an exercise changing the code so that the play-seq "remembers" the tone pitch and duration given at call time. What I found out when trying to solve this was that callback isn't aware of nested closures. By simply encapsulating the example like this:

(define call
  (lambda ()
    (define play-seq
      (lambda (time plst rlst)
        (play-note time synth (car plst) 80 (car rlst))
        (callback (+ time (* .5 (car rlst))) 'play-seq (+ time (car rlst))
                  (if (null? (cdr plst))
                      '(60 62 65 69 67)
                      (cdr plst))
                  (if (null? (cdr rlst))
                      '(11025 11025 22050 11025)
                      (cdr rlst)))))
    (play-seq (now) '(60 62 65 69 67) '(11025 11025 22050 11025))))

(call)

what happens is either:

  1. you've evaluated only the modified example, in which case only one tone is heard and the REPL throws an error:
    eval: unbound variable: play-seq
    Trace: callback-adapter
  2. or you've evaluated the modified version after evaluating the original, in which case things seem to work find but what actually happens is that the original play-note is passed to the callback call and the rest continues there.
benswift commented 4 years ago

Hmm, that's true. Looks like callback isn't finding play-seq in the enclosing environment; only looking in the top-level one.

Since callback is an Extempore thing (obviously---RnRS don't have temporal semantics) and it has a bunch of special-cased behaviour for dealing with the callback closure, which can specified either by passing a closure directly, or through a symbol which is should eval to a closure.

So, as a workaround, you can pass play-seq as the second argument to callback (instead of the quoted symbol 'play-seq and it'll work.

I can have a look into it when I get the chance, but I'd say that it might be a bit fiddly to handle this better. I don't dispute that the current behaviour isn't ideal, but it's probably not a super-high priority bugfix (esp. considering the workaround). Thanks for doing the detective work, though :)

adql commented 4 years ago

Thanks for the workaround, I wasn't aware of it.

It wasn't much detective work ;) I was trying to solve the exercise in Note-level music (just search "exercise", it's the first one that comes up), where using an internal iteration procedure that will refer back to the original note/duration lists at the end of every iteration was the "obvious" way to go, especially having read a significant portion of SICP...

It works now, but alas! There is no way to stop the noise! :D Since the callback is to an internal closure of play-seq, it's impossible to redefine it from the top-level environment (as far as I understand). I guess this is not the way to do things with callback at the first place.

benswift commented 4 years ago

Yeah, fair enough.

To be honest, what I had in mind with that exercise was to keep passing the whole list through but rotateing it by 1 each time---that's a nice trick that I use often. Alternately, you can specify another argument (the "full" list) which just gets passed back unchanged each time, which you can use to "refresh" the list when plst is empty. Your way isn't dumb, it's just not really a use case that was accounted for in the design of callback.

digego commented 4 years ago

Yeah, just to follow through on this - this isn't a bug - and doesn't require a workaround (well the tutorial might - but extempore doesn't). The symbol version of a TR is specifically there to resolve top level closures only. The non-symbol version passes a closure which cannot be changed once scheduled. This is an important (and useful) distinction. They are two different modalities, and each has its uses. It's just that most of the time, most of the people, will be using and expecting the behaviour of the symbol version. And don't forget scheduled continuations, which is where things really get interesting. You might find this worth a read:

http://extempore.moso.com.au/temporal_recursion.html

One thing you might want to consider though is that you can eval to the top-level environment like so (eval '(define something "something") (interaction-environment)). The code below does what I think you originally intended (untested so might need tweaking).

(define call (lambda () (eval '(define play-seq (lambda (time plst rlst) (play-note time synth (car plst) 80 (car rlst)) (callback (+ time (* .5 (car rlst))) 'play-seq (+ time (car rlst)) (if (null? (cdr plst)) '(60 62 65 69 67) (cdr plst)) (if (null? (cdr rlst)) '(11025 11025 22050 11025) (cdr rlst))))) (interaction-environment)) (play-seq (now) '(60 62 65 69 67) '(11025 11025 22050 11025))))

(call)

;; and stop (define play-seq)

Cheers, Andrew.

On Sun, May 3, 2020 at 1:28 PM Ben Swift notifications@github.com wrote:

Yeah, fair enough.

To be honest, what I had in mind with that exercise was to keep passing the whole list through but rotateing it by 1 each time---that's a nice trick that I use often. Alternately, you can specify another argument (the "full" list) which just gets passed back unchanged each time, which you can use to "refresh" the list when plst is empty. Your way isn't dumb, it's just not really a use case that was accounted for in the design of callback.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/digego/extempore/issues/377#issuecomment-623048379, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEHPKJHILEEORRY4M53R3TRPTQGDANCNFSM4MG3QOCQ .

adql commented 4 years ago

Oh dear, when I wrote my comment here two days ago I completely forgot that I had already given all these details about the exercise at the beginning... sorry for that.

Thanks for the answers, Ben and Andrew. I think it's getting more clear now. I've also read the part regarding temporal recursion in Andrew's thesis since then. One simple thing that helped removing some confusion was understanding that callback is actually an alias for schedule. So far I had thought of callback as something that goes back to the last recursion point (a bit like a temporal version of Clojure's recur), so I wasn't sure why providing the symbol of the closure to be called was necessary at all. With this concept in mind, calling back the immediate nested closure was an obvious thing. But thinking about it as a general scheduler, whose idiomatic use is as a callback, made things more clear. It might be worth emphasizing this aliasing fact in the docs (particularly this one, which is an entry point for newcomers).