Support other variations in feedback syntax

[countvajhula]

Summary of Changes

This is WIP to fix #34 .

Things remaining:

• confirm that the new variations can always be interpreted unambiguously
• what to do about the identifier case, feedback? Should it accept N or while?
• Depending on the answers above, would it make sense to have distinct feedback and feedback-while forms?

Public Domain Dedication

• In contributing, I relinquish any copyright claims on my contribution and freely release it into the public domain in the simple hope that it will provide value.

[benknoble]

• The patterns in the docs don't look ambiguous to me, but that doesn't mean the implementation tracks (I didn't look too closely).
• I don't have a strong opinion or comment about N vs. while.

[countvajhula]

@benknoble Thanks for taking a look! Much appreciated.

I don't really have much clarity on this one, and I'm not sure it addresses the original observations by @1e1001 , even if it does make the syntax a little more flexible.

Though now that I'm writing this down, I'm wondering if adding a (feedback flo) version would address it, which would consume the first input as N, something like (~> (3 5) (feedback add1)) ; => 8. That might help with 1e1001's original implementation.

But also, using Racket variables and syntax is another way, as in the final solution here: https://github.com/countvajhula/qi/wiki/Design-Challenge-%231#1e1001-qi-using-feedback---2 . In cases where that isn't applicable, it may be that having Qi-native bindings (i.e. instead of using let and define, a Qi form like (as result)) would help here, and e.g. it may be possible to do (~> (3 5) (== (as N) _) (feedback N add1)) ; => 8. I don't really know how Qi bindings will work yet, but we could aim for this to be part of the design in #36 if we think it makes sense.

Re: while vs N, ok good to know -- the former is how it works currently, so maybe we just leave it alone for now unless we come up with a clear reason to change it.

[countvajhula]

I just realized that:

(~> ... feedback)

can always be rewritten to:

(~> ... (feedback (while ...)))

... since the while condition is going to operate in the context of runtime values in either case (in addition to having access to the lexical (static) context in which the feedback form is used).

On the other hand, with (feedback N), the expression N cannot be in terms of runtime inputs as it only has access to the lexical / static context.

In the light of this, it seems that it would be better for the default identifier-only behavior to be N rather than while, since the latter can be written a different (the above) way!

I've made this change.

Using the new "N" semantics of the identifier form of feedback, 1e1001's original implementation can be simplified to:

(define-flow recurse
  (~> (-< 1>
          (gen (☯ _))
          (~> (select 2 3) X clos))
      feedback))
(recurse 3 5 *)

This change is backwards incompatible, FTR.

Other changes in this PR:

• there were many implementations of feedback in different matching rules. I changed it so that there are only two implementations -- one for "N" and one for "while", and the other rules now rewrite to feedback to leverage one of these two core implementations.
• these core implementations are more performant than the ones that were there before, I believe. At least the identifier-only case is dramatically faster.
• made a couple of ambient changes to simplify if and clos.

[countvajhula]

One more change: the advantage of using a then form in feedback is to "clean things up" after the looping is done, in order to produce just the desired output instead of any control or scratch values. This keeps all of the feedback-related logic internal to the feedback form. But when used in identifier form, there's nothing "internal" to the feedback form in any case, and so, having the then flow be an input to the feedback form doesn't make as much sense anymore, especially when it is often going to be (flow _).

With this last change, 1e1001's example becomes:

(define-flow recurse
  (~> (group 1 _ (~> X clos))
      feedback))
(recurse 3 5 *)

[benknoble]

That last solution in particular looks quite elegant.

My only feedback so far is that some of the generated syntax is quite large (lambdas, loops, etc.) that might be better off in a function (as in https://docs.racket-lang.org/style/Language_and_Performance.html#%28part._.Macros__.Space_and_.Performance%29). But that can be left for later; I didn't look to see what all the other forms were doing.

[countvajhula]

@benknoble I've made the change! Out of curiosity, I reverted the identifier behavior to the original "while" semantics and then ran the benchmark for feedback before and after changing it to use functions instead of direct expansions (so that it's an apples to apples comparison), and it appears there's a significant difference in performance:

Before:
$ racket profile/forms.rkt -f feedback
feedback: 274 ms
$ racket profile/forms.rkt -f feedback
feedback: 277 ms
$ racket profile/forms.rkt -f feedback
feedback: 239 ms
$ racket profile/forms.rkt -f feedback
feedback: 268 ms
$ racket profile/forms.rkt -f feedback
feedback: 266 ms

After:
$ racket profile/forms.rkt -f feedback
feedback: 246 ms
$ racket profile/forms.rkt -f feedback
feedback: 148 ms
$ racket profile/forms.rkt -f feedback
feedback: 138 ms
$ racket profile/forms.rkt -f feedback
feedback: 142 ms
$ racket profile/forms.rkt -f feedback
feedback: 139 ms

I didn't realize this could make such a difference 🤭 I'm still not really sure I understand why. Thank you for pointing it out!

[benknoble]

I suspect the compiler has a better chance to optimize the internals of a single, named function rather than the same code expanded all over the place. (I once read the compiler doesn't optimize anonymous functions at all.)

[countvajhula]

That sounds plausible!

Thanks for reviewing, I will merge 🙏