fixing fpsWhen in Elm-0.15

[jvoigtlaender]

Some evidence that fpsWhen is not working at the moment in 0.15, maybe due to the JS type error fixed by this first commit:

import Time
import Graphics.Element

main = Varying.map Graphics.Element.show <|
       Stream.fold (+) 0 <|
       Time.fpsWhen 60
         (Varying.map (\c -> c > 100) (Stream.fold (\_ c -> c+1) 0 (Stream.fromVarying (Time.every 100))))     

This is not updating any longer after 100ms. The same when replacing (\c -> c > 100) by (\c -> c < 100). In neither case that seems to be the correct behavior.

[jvoigtlaender]

Actually, I think the whole fpsWhen function is quite broken now. It has different behavior than the version from the beginning of February, which we arrived at (the semantics) after a few rounds of discussion including @jwmerrill. One difference, if I understand the current version correctly, is that when the isOn input signal switches from False to True, fpsWhen does not now have an event at all, whereas previously it would emit an event at that point, with delta value 0.

Apart from these semantic points, the current implementation looks simply unsafe. Not all cases are covered: the fpsUpdate function may terminate without a return value, because the three conditions tested in it do not exhaust all possibilities. The dropRepeats in there: https://github.com/elm-lang/core/blob/42c52cd0ec26bbe4fa296cc2006fb6c2033f5a2b/src/Native/Time.js#L28 was quite essential.

[jvoigtlaender]

Actually, I think the whole fpsWhen function is quite broken now. It has different behavior than the version from the beginning of February, which we arrived at (the semantics) after a few rounds of discussion including @jwmerrill. One difference, if I understand the current version correctly, is that when the isOn input signal switches from False to True, fpsWhen does not now have an event at all, whereas previously it would emit an event at that point, with delta value 0.

Apart from these semantic points, the current implementation looks simply unsafe. Not all cases are covered: the fpsUpdate function may terminate without a return value, because the three conditions tested in it do not exhaust all possibilities. The dropRepeats in there: https://github.com/elm-lang/core/blob/42c52cd0ec26bbe4fa296cc2006fb6c2033f5a2b/src/Native/Time.js#L28 was quite essential.

[jwmerrill]

Unless we start testing these semantics, I think we are doomed to keep letting them slip over time. Several of us discussed and agreed about the need for and desire for tests of this functionality in the last round, but none of us got around to implementing any.

As far as I know, there aren't any "integration" tests of Elm yet that test for correct behavior of the native runtime. It'll be a push to get some infrastructure in place to do tests like that, but once we have the first one it will be much easier to add more incrementally.

[jwmerrill]

Unless we start testing these semantics, I think we are doomed to keep letting them slip over time. Several of us discussed and agreed about the need for and desire for tests of this functionality in the last round, but none of us got around to implementing any.

As far as I know, there aren't any "integration" tests of Elm yet that test for correct behavior of the native runtime. It'll be a push to get some infrastructure in place to do tests like that, but once we have the first one it will be much easier to add more incrementally.

[evancz]

I was not certain I got things correct, but I felt that if we have the ability to talk about Stream, it makes sense for fps to be giving time deltas. When fpsWhen starts, the next event should be the first reasonable time delta. Does this make sense as a goal?

[evancz]

I was not certain I got things correct, but I felt that if we have the ability to talk about Stream, it makes sense for fps to be giving time deltas. When fpsWhen starts, the next event should be the first reasonable time delta. Does this make sense as a goal?

[jvoigtlaender]

About testing these semantics generally, yes, that would be important. Unfortunately, I don't have capacity to work on a solution for that.

About the semantics of this specific function now:
In an earlier discussion, we looked at a specific case, where isOn has the following behavior:

• initial value False
• at 0.5 seconds, has a True event
• at 2.0 seconds, has a False event
• at 3.5 seconds, has a True event
• at 4.0 seconds, has a False event
• no further events

The version of fpsWhen created at the end of that discussion had the following semantics, for fpsWhen 1 isOn:

• at 0.5 seconds, has an event with value 0
• at around 1.5 seconds, has an event with value around 1000
• at 2.0 seconds, has an event with value around 500
• at 3.5 seconds, has an event with value 0
• at 4.0 seconds, has an event with value around 500
• no further events

Of course, these were already time deltas. But if I understand Evan correctly, he is proposing to not have the 0 valued events at 0.5 and 3.5 seconds. I think that is something one could agree upon. But actually, the current implementation also does not send the 500 valued events at 2.0 and 4.0 seconds. And I think this is really wrong. It means that in total the deltas will not sum up to the duration of isOn being on.

So I think at least two further changes are in order to the current version:

• A call of dropRepeats should be wrapped around isOn in https://github.com/elm-lang/core/blob/master/src/Native/Time.js#L27.
• Line https://github.com/elm-lang/core/blob/master/src/Native/Time.js#L73 should look like line https://github.com/elm-lang/core/blob/master/src/Native/Time.js#L55.

Moreover, I do not get the reason for having delay 0 in https://github.com/elm-lang/core/blob/master/src/Native/Time.js#L39, rather than delay msPerFrame. The latter would seem more in line with Evan's proposal of only sending the first delta once some actual time has passed.

[jvoigtlaender]

About testing these semantics generally, yes, that would be important. Unfortunately, I don't have capacity to work on a solution for that.

About the semantics of this specific function now:
In an earlier discussion, we looked at a specific case, where isOn has the following behavior:

• initial value False
• at 0.5 seconds, has a True event
• at 2.0 seconds, has a False event
• at 3.5 seconds, has a True event
• at 4.0 seconds, has a False event
• no further events

The version of fpsWhen created at the end of that discussion had the following semantics, for fpsWhen 1 isOn:

• at 0.5 seconds, has an event with value 0
• at around 1.5 seconds, has an event with value around 1000
• at 2.0 seconds, has an event with value around 500
• at 3.5 seconds, has an event with value 0
• at 4.0 seconds, has an event with value around 500
• no further events

Of course, these were already time deltas. But if I understand Evan correctly, he is proposing to not have the 0 valued events at 0.5 and 3.5 seconds. I think that is something one could agree upon. But actually, the current implementation also does not send the 500 valued events at 2.0 and 4.0 seconds. And I think this is really wrong. It means that in total the deltas will not sum up to the duration of isOn being on.

So I think at least two further changes are in order to the current version:

• A call of dropRepeats should be wrapped around isOn in https://github.com/elm-lang/core/blob/master/src/Native/Time.js#L27.
• Line https://github.com/elm-lang/core/blob/master/src/Native/Time.js#L73 should look like line https://github.com/elm-lang/core/blob/master/src/Native/Time.js#L55.

Moreover, I do not get the reason for having delay 0 in https://github.com/elm-lang/core/blob/master/src/Native/Time.js#L39, rather than delay msPerFrame. The latter would seem more in line with Evan's proposal of only sending the first delta once some actual time has passed.

[jvoigtlaender]

Oh, I just notice that dropRepeats does not exist anymore. An oversight?

[jvoigtlaender]

Oh, I just notice that dropRepeats does not exist anymore. An oversight?

[jvoigtlaender]

Consider this quote from the current fpsWhen documentation:

"The first time delta after a pause is always zero, no matter how long the pause was. This way summing the deltas will actually give the amount of time that the output signal has been running."

The statements of both sentences are wrong with the current implementation (apart from the fact that the current implementation can probably crash completely or have undefined behavior, because fpsUpdate does not always have a return value).

[jvoigtlaender]

Consider this quote from the current fpsWhen documentation:

"The first time delta after a pause is always zero, no matter how long the pause was. This way summing the deltas will actually give the amount of time that the output signal has been running."

The statements of both sentences are wrong with the current implementation (apart from the fact that the current implementation can probably crash completely or have undefined behavior, because fpsUpdate does not always have a return value).

[jvoigtlaender]

Let me make a somewhat radical proposal: dropping fpsWhen.

What are actual use cases for this signal combinator? Isn't it actually a bit against the spirit of the "Architecture in Elm" material?

In my arguing for the inclusion of Maybe.isNothing(PR #164), I searched through GitHub repos for uses of that function. Anyone feeling like doing the same for fpsWhen and checking whether there are real uses (apart from just examples/tutorials demonstrating fpsWhen for the sake of it, simply because it currently is in the standard lib)?

I suggest:

• Drop fpsWhen.
• Keep fps, with a very simple implementation.
• If at some point in the future a real need or use case for something like fpsWhen arises, have a discussion about what the exact semantics should be. So, let the stakeholders at that point (the people who really need fpsWhen) discuss whether or not a 0 valued event should be sent at the time isOn switches from False to True, etc. Then, make an implementation informed by that discussion.

[jvoigtlaender]

Let me make a somewhat radical proposal: dropping fpsWhen.

What are actual use cases for this signal combinator? Isn't it actually a bit against the spirit of the "Architecture in Elm" material?

In my arguing for the inclusion of Maybe.isNothing(PR #164), I searched through GitHub repos for uses of that function. Anyone feeling like doing the same for fpsWhen and checking whether there are real uses (apart from just examples/tutorials demonstrating fpsWhen for the sake of it, simply because it currently is in the standard lib)?

I suggest:

• Drop fpsWhen.
• Keep fps, with a very simple implementation.
• If at some point in the future a real need or use case for something like fpsWhen arises, have a discussion about what the exact semantics should be. So, let the stakeholders at that point (the people who really need fpsWhen) discuss whether or not a 0 valued event should be sent at the time isOn switches from False to True, etc. Then, make an implementation informed by that discussion.

[evancz]

The documentation did not get updated to match what I was going for, my goal was:

• When it goes on, no event triggers.
• Time passes, resulting in events indicating the time deltas.
• When it goes off, you get the last bit of time. (broken now?)

On the question of deleting it altogether, there are two things to think about.

1. 0.15 will be the first release ever to make it really easy to loop information back, so turning FPS on and off dependent on your application state will be possible. I think that makes fpsWhen much more useful.
2. It may be possible to do all this stuff with promises. Spawn a thread that can use Promise.sleep and get messages from other threads, it then hears about on/off messages somehow. It might be interesting to explore this to see how close we are to doing it.

[evancz]

The documentation did not get updated to match what I was going for, my goal was:

• When it goes on, no event triggers.
• Time passes, resulting in events indicating the time deltas.
• When it goes off, you get the last bit of time. (broken now?)

On the question of deleting it altogether, there are two things to think about.

1. 0.15 will be the first release ever to make it really easy to loop information back, so turning FPS on and off dependent on your application state will be possible. I think that makes fpsWhen much more useful.
2. It may be possible to do all this stuff with promises. Spawn a thread that can use Promise.sleep and get messages from other threads, it then hears about on/off messages somehow. It might be interesting to explore this to see how close we are to doing it.

[evancz]

Also, I want to stress that my goal here was to try out a new kind of type signature, so I messed with things so it'd work. It may be that we can go to pretty much the same implementation as before, my goal was just to do an experiment!

[evancz]

Also, I want to stress that my goal here was to try out a new kind of type signature, so I messed with things so it'd work. It may be that we can go to pretty much the same implementation as before, my goal was just to do an experiment!

[jvoigtlaender]

About 0.15 making fpsWhen more useful than before, yes, that sounds relevant.

About the current implementation, yes, I think it indeed breaks "When it goes off, you get the last bit of time.", and it does not correctly handle the case that isOn has repeated events (e.g., when the timer is already on, but another True arrives on isOn).

I'll have a go at proposing fixes for these issues, by adding commits to this PR.

[jvoigtlaender]

About 0.15 making fpsWhen more useful than before, yes, that sounds relevant.

About the current implementation, yes, I think it indeed breaks "When it goes off, you get the last bit of time.", and it does not correctly handle the case that isOn has repeated events (e.g., when the timer is already on, but another True arrives on isOn).

I'll have a go at proposing fixes for these issues, by adding commits to this PR.

[jvoigtlaender]

I think this is now okay and has the semantics Evan suggested above.

[jvoigtlaender]

I think this is now okay and has the semantics Evan suggested above.

[jvoigtlaender]

I'm not sure whether this fix is still relevant for the 0.15 release. On the one hand, with streams gone, maybe 0.15 will ship the old fpsWhen code? On the other hand, Time.js was not rolled back in any way yet. In fact, currently it uses a streamMap function from Native.Signal that does not actually exist in there.

[jvoigtlaender]

I'm not sure whether this fix is still relevant for the 0.15 release. On the one hand, with streams gone, maybe 0.15 will ship the old fpsWhen code? On the other hand, Time.js was not rolled back in any way yet. In fact, currently it uses a streamMap function from Native.Signal that does not actually exist in there.

[evancz]

I think it makes sense to revert Time.js back to how it was in 0.14 with the implementation you and @jwmerrill came up with. I have not gotten to it yet! Can you try to do that?

[evancz]

I think it makes sense to revert Time.js back to how it was in 0.14 with the implementation you and @jwmerrill came up with. I have not gotten to it yet! Can you try to do that?

[jvoigtlaender]

Sure. Will do tomorrow.

This PR here can thus be closed. PR https://github.com/elm-lang/core/pull/209 contains the same fixes, but retargeted towards the future branch.

[jvoigtlaender]

Sure. Will do tomorrow.

This PR here can thus be closed. PR https://github.com/elm-lang/core/pull/209 contains the same fixes, but retargeted towards the future branch.

[evancz]

Okay, I tried to get it back in order in the interest of getting a draft of 0.15 to people, but definitely glance through if you get a chance!

[evancz]

Okay, I tried to get it back in order in the interest of getting a draft of 0.15 to people, but definitely glance through if you get a chance!

[jvoigtlaender]

Did so. See PR https://github.com/elm-lang/core/pull/210.

[jvoigtlaender]

Did so. See PR https://github.com/elm-lang/core/pull/210.