Fancy throttle

[3noch]

No description provided.

[lspitzner]

This reminds me of #184 which has some similar constructs.

Also, at a glance I wonder if there is much use to using Semigroup if you are not forcing the intermediate value.

[ElvishJerricco]

That is a fancy throttle! Nice!

[lspitzner]

Does this behave as expected for a delay of 5s and a example input signal that fires once every second? If I am reading things correctly, the delayed signals are not filtered, meaning that after 5 seconds, the delayed signal fires every second too, which turns immediate mode back on at the same rate. So you get 1 output, 5s pause, 4 merged outputs, and then every (other?) second some output. Whether it is every second or every other probably is not deterministic due to continuous-time/fuzzy delays.
(edit: i did not read things correctly :p)

I'd expect proper 5s intervals of silence, as you'd get if only the non-immediate-mode-entering signals trigger, after a delay, the back-to-immediate-mode switch.

[ali-abrar]

@3noch (maybe a question for James) do we have a test that would answer @lspitzner's question?

We should definitely figure out how this interacts/overlaps with #184 before merging.

@cgibbard may also have some thoughts as the throttle code here seems to be an upgraded version of something he wrote (that somehow never made it to develop..)

reflex/src/Reflex/Time.hs

Line 243 in 4bdf228

throttle :: (MonadFix m, MonadHold t m, PerformEvent t m, TriggerEvent t m, MonadIO (Performable m)) => NominalDiffTime -> Event t a -> m (Event t a)

[3noch]

cc @xplat

[xplat]

Does this behave as expected for a delay of 5s and a example input signal that fires once every second? If I am reading things correctly, the delayed signals are not filtered, meaning that after 5 seconds, the delayed signal fires every second too, which turns immediate mode back on at the same rate. So you get 1 output, 5s pause, 4 merged outputs, and then every (other?) second some output. Whether it is every second or every other probably is not deterministic due to continuous-time/fuzzy delays.

I'd expect proper 5s intervals of silence, as you'd get if only the non-immediate-mode-entering signals trigger, after a delay, the back-to-immediate-mode switch.

I had exactly the same question after reading the code for throttle. @cgibbard gave me the answer -- the delay is fired based on the output events, not the input events. So the first output, 5s pause, 4 merged outputs, and then nothing until 5 seconds after the 4 merged outputs.

It's all twisty, but it works.

[lspitzner]

Right, i should have noticed that. sorry. Looks fine to me!

(and regarding functionality overlap: I think it is worth having this function in addition to the #184 ones. Even if you could express fancy-throttle in terms of gateGather, as I suspect you can.)

[sboosali]

should we add that clarification to the documentation?

…

On Tue, Nov 6, 2018, 16:15 Lennart Spitzner ***@***.*** wrote: Right, i should have noticed that. sorry. Looks fine to me! (and regarding functionality overlap: I think it is worth having this function in addition to the #184 <#184> ones. Even if you could express fancy-throttle in terms of gateGather, as I suspect you can.) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#242 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACNoMc6BVUEr9kI3tpVN0tPV1i73ZOTMks5usiY0gaJpZM4YPUmb> .

[ali-abrar]

Given that the question has come up a couple times, I think we should On Tue, Nov 6, 2018, 7:44 PM Spiros Boosalis <[email protected]> wrote:

…

should we add that clarification to the documentation? On Tue, Nov 6, 2018, 16:15 Lennart Spitzner ***@***.*** wrote: > Right, i should have noticed that. sorry. Looks fine to me! > > (and regarding functionality overlap: I think it is worth having this > function in addition to the #184 > <#184> ones. Even if you could > express fancy-throttle in terms of gateGather, as I suspect you can.) > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#242 (comment)>, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ACNoMc6BVUEr9kI3tpVN0tPV1i73ZOTMks5usiY0gaJpZM4YPUmb > > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#242 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHFpRriLjMa-01ObjivOW818vGmiDkasks5usizpgaJpZM4YPUmb> .

[matthewbauer]

It would be good to get this merged.

[luigy]

pinging @cgibbard since I believe you were somewhat involved in this? afaik this has been in use in prod for some time

[3noch]

The imports were alphabetized prior to this change

[tomsmalley]

throttleBatchWithLag pure causes a hang which leads to a stack overflow. Granted, the docs say that the "lag" function should construct an event, but we should at least give a stronger warning there.

[3noch]

@tomsmalley Yikes. I wonder if there's some "innocuous" operation we can do to ensure pure doesn't work. Something like delay 0 =<< but I don't know if that would cause unintended consequences.

[3noch]

Actually I'm sure it would have bad consequences...but perhaps something else.