Discrepancy in model and implementation about reactive-banana HOT 15 OPEN

I do not understand why external switching would cause changes to the event.

In fact, I found the discrepancy between the Model and the Combinators module.

import Reactive.Banana.Combinators as C
testCase = [Just n | n <- [0..5]]
listingE event = C.accumE [] ((\x -> (++ [x])) <$> event)
evenE event = C.filterJust $ (\n -> n <$ guard (even n)) <$> event
nested event = C.switchE never (C.observeE (listingE event <$ evenE event))
C.interpret nested testCase

gives
[Nothing,Just [1],Just [1,2],Just [3],Just [3,4],Just [5]]

while

import Reactive.Banana.Model qualified as M
testCase = [Just n | n <- [0..5]]
listingE event = M.accumE [] ((\x -> (++ [x])) <$> event)
evenE event = M.filterJust $ (\n -> n <$ guard (even n)) <$> event
nested event = M.switchE M.never (M.observeE (listingE event <$ evenE event))
M.interpret nested testCase

gives
[Nothing,Just [0,1],Just [0,1,2],Just [2,3],Just [2,3,4],Just [4,5]]
as I expected.

I wonder which one is implemented wrong.

EDIT: Failing test

from reactive-banana.

Huh, yes, I see what you mean now. Sorry it took so long for me to understand. I agree with your assessment; the model seems correct to me, too.

from reactive-banana.

Oh, I think that's a puzzle for @HeinrichApfelmus or another intrepid adventurer.

If I had to speculate, it seems we're building the event too late. We ought to build it the moment it's switched to, even if we only start observing its values in the next.

... wait, heh, I think I'm starting to lean towards the model being incorrect. The preceding paragraph seems a little nonsense - we delay rebuilding the network until the end of a moment because it's inefficient to do it "live".

But either way, great find!

from reactive-banana.

The comment on switchE is wrong. My guess is it simply wasn't properly fixed up (by me) whenever we added the initial event argument.

It's trying to convey that at the moment the event-of-events fires, if the event that it contains also fires, that inner event's value will not be present.

In plainer English: switchE e es acts like e up until (and including) the moment es fires, at which point it starts acting like the event that es fired, and so on.

Hope that helped somewhat. In my opinion, we should maybe rework the "semantics" comment of switchE. It's just a bit too complicated to extract meaning from. A nice image or something would be a lot clearer.

from reactive-banana.

I see.
While trying to check the semantics, I noticed strange behavior regarding observeE and accumE.

Consider this code:

testCase = [Just n | n <- [0..5]]
listing event = accumE [] ((\x -> (++ [x])) <$> event)
nested event = switchE never (observeE (listing event <$ filterE even event))
interpret nested testCase

This gives
[Nothing,Just [1],Just [1,2],Just [3],Just [3,4],Just [5]]

Note that filterE even event fires at [Just 0, Nothing, Just 2, Nothing, Just 4, Nothing].
The event parameter itself fires at the moment time,
so observeE (listing event <$ filterE even event) should give alike
[Just [Just [0], Just [0, 1], ..], Nothing, Just [Just [2], Just [2, 3], ..]], .. ] :: Event (Event [Int])

However, when collapsed with switchE, we can see that e.g. [2, 3] is missing:
[Nothing,Just [1],Just [1,2],Just [3],Just [3,4],Just [5]].

I guess accumE is dropping event occurrence simultaneous with the moment, but this could be wild guess.

from reactive-banana.

That output looks right to me. Could you explain why [2,3] should appear?

from reactive-banana.

I should have stayed with the list semantics..
Let's say event = [(Time n, n) | n <- [0..5]].
I believe that docs imply accumE will include the simultaneous event occurrence.
So, e.g. listingE event (Time 2) = [(Time 2, [2]), (Time 3, [2, 3]), (Time 4, [2, 3, 4]), (Time 5, [2, 3, 4, 5])].
As filterE even event also happens at Time 2, listingE event <$ filterE even event contains (Time 2, listingE event).
Thus, we have

observeE (listingE event <$ filterE even event)
  = [.., (Time 2, listingE event (Time 2) ,..]
  = [.., (Time 2,  [(Time 2, [2]), (Time 3, [2, 3]), (Time 4, [2, 3, 4]), (Time 5, [2, 3, 4, 5])] ), ..]

Then, switchE switches after each event occurrence, so we should have

switchE never (observeE (listingE event <$ filterE even event))
  = [..,  (Time 3, [2, 3]), (Time 4, [2, 3, 4]), ..]

so I think the fourth element should be Just [2, 3].

from reactive-banana.

At (the end of the moment at) time 2, we switch to a new observeE (listingE event), which first emits at time 3 with value [3]. The event doesn't exist at time 2, therefore it doesn't accumulate a [2]. Does that make sense?

from reactive-banana.

So, what needs to be fixed to correct the semantics?

from reactive-banana.

I think we just need to rewrite the comment on switchE to treat the initial event specially, because its first occurrence is visible in the output, unlike the others. Is that what you mean? Or maybe you understood that's what I meant by this comment, but you see a mistake with that explanation?

In plainer English: switchE e es acts like e up until (and including) the moment es fires, at which point it starts acting like the event that es fired, and so on.

from reactive-banana.

Oh, I think that would be fine for a documentation, I was commenting on fixing the Failing test.

from reactive-banana.

This note seems relevant:

{-
* Note [PulseCreation]

We assume that we do not have to calculate a pulse occurrence
at the moment we create the pulse. Otherwise, we would have
to recalculate the dependencies *while* doing evaluation;
this is a recipe for desaster.

-}

from reactive-banana.

Does the Moment here means in logical time, or something else?

from reactive-banana.

Yes, the logical time

from reactive-banana.

If I understood correctly, switchE documentation is right while once and accumE need to be fixed. Correct?
Is this an wrong assessment?

from reactive-banana.