search

turion / rhine

Haskell Functional Reactive Programming framework with type-level clocks
http://hackage.haskell.org/package/rhine
121 stars 21 forks source link

Add ClSF pre-/post-composition #167

Closed turion closed 2 years ago

turion commented 2 years ago

I don't have any tests yet, but we can test on #165 .

turion commented 2 years ago

@jmatsuhita can you give this a quick review?

jmatsushita commented 2 years ago

Hey @turion, I'm taking a look at this and wonder a few things:

Don't know if any of this makes sense or could lead to some simplifications, but I thought I'd ask and learn :)

Otherwise LGTM, since it does address how to consume a Rhine m (ParallelClock m clL clR) a (Either b c) which is great!

turion commented 2 years ago 
* Should we also be able to pre-/post- compose a `Rhine` with an `SN` if the inputs and outputs align?

No, that wouldn't work. A Rhine is an SN with a clock, so that SN you'd compose doesn't have a clock.

* Is the fact that we have to add 2 constructors to `SN` a sign that there might be multiple structures in there that could be decoupled or maybe made more explicit? Something like a category on clocks with a category on data types.

Yes, probably some simplification of Synchronous, Sequential, Precompose and Postcompose can be done. I'd expect something like a heterogeneous list with clocks interspersing it. But it's not the biggest problem since there are few functions that need to operate on SNs directly. (In comparison, the API of Rhine is in much bigger need of simplification.)

* Have you tried, and would it be possible or desirable, to avoid the `In` `Out` type families by explicitly making `SN` carry input and output clock types as `SN m aCl bCl a b`?

Yes, I considered that. It would probably still need the type of the full clock. The cool thing about your proposal might be that we could maybe recover an Arrow instance for SNs or Rhines, and maybe get rid of all the funny combinators that noone can remember. I tried this a few times with different approaches and got stuck every time so far. I might try again :D

Otherwise LGTM, since it does address how to consume a Rhine m (ParallelClock m clL clR) a (Either b c) which is great!

The only thing I'm unsure about is whether this introduces a bug in your terminal application :thinking: (what you reported in https://github.com/turion/rhine/pull/165#issuecomment-1159787086)

turion commented 2 years ago

The only thing I'm unsure about is whether this introduces a bug in your terminal application thinking (what you reported in https://github.com/turion/rhine/pull/165#issuecomment-1159787086)

Ok, pretty sure this is not the case.

turion commented 2 years ago

@jmatsushita see https://github.com/turion/rhine/commit/62f17f4786e0a1fe7c0c245f83fd16c1f46bdafd where I indeed tried your proposal. Maybe it can lead somewhere. But I need to finish https://github.com/turion/rhine/pull/171 first, it will simplify most other changes.