Closed stevekrouse closed 5 years ago
I definitely agree that time.log()
should not fail silently.
A pull behavior can change infinitely often and it's hard to say exactly how often log
should sample the behavior.
What about changing the type of log
into:
log(prefix?: string, sampleEvery: number = 1000): Behavior<A>;
It takes second optional argument which is how many milliseconds to wait between every sampling. A default of 1000 would sample every second and by specifying the argument this can be changed.
This would solve the problem. Very reasonable.
Something about this doesn't feel right in my gut though. It feels like pull-based behaviors a really different kind of thing here. Would it be crazy to have a separate type for them?
This issue now also tracks the fact that changes(time)
also fails silently
Conal says that you semantically shouldn't be able to tell when a behavior changes. I think changes
and log
and when
should not be allowed on Behaviors. It's breaking the semantic model to have them work on push-based behaviors.
The changes
function is very very useful. It's the only way to go from a Behavior
into a Stream
.
I'm not sure what statement from Conal Elliott you are referring to. But, if we're thinking of the same thing then I think what Conal meant was that you shouldn't be able to know when a behavior changes in an internal way that stems from the implementation.
But, I don't think there is a problem with our changes
. I see it as a fairly similar to whenJust
from the FRP Now paper and predicate
from Conal's Functional Reactive Animation paper. The former lets you know when a behavior changes from Nothing
to Just
and the later lets you know when a predicate on the value of a behavior becomes true. I see changes
as a generalization of both of these.
Also, I made an attempt at writing down the semantics for changes
at one point. It's a bit hairy but it looks like this.
Here inf
denotes the least upper bound of the set and the limit is there for technical reasons (basically to make changes
play nice together with scan
and stepper
).
Hm, that's a good point that predicate
seems to have quite similar semantics. I don't know what he meant exactly then...
Back to the original problem: specifying a sample interval feels wrong to be. I feel like its the library's job to figure out how often sampling should happen with some interval analysis. You know which parts of a behavior are pull-based, so you could start by sampling them a lot (once per milisecond) but then maybe back off it it seems to be stable for a while.
Time-based behaviors change quite predictably, which should help.
Would it be impractical to give people what they ask for if they do time.log()
or changes(time)
? You'd realize quite quickly that's not what you want, but I think it'd be good feedback regardless. Or would that just crash people's computers? If it'd crash computers, maybe those functions return an runtime error if you ask for the log/changes of something that changes too quickly?
Back to the original problem: specifying a sample interval feels wrong to be. I feel like its the library's job to figure out how often sampling should happen with some interval analysis.
Not sure I follow here. What do you want to happen if you do time.log()
? It seems to me like there are perfectly valid use cases both for wanting both very frequent logging, if you're debugging an animation perhaps, and for wanting much rarer logging if you're debugging an alarm clock for instance.
Would it be impractical to give people what they ask for if they do
time.log()
orchanges(time)
? You'd realize quite quickly that's not what you want, but I think it'd be good feedback regardless.
In the changes(time)
case I think we agree :smile: Hareactive should support that to the extend possible. We have an issue open about that: https://github.com/funkia/hareactive/issues/21. We should do whatever we can to make that operation work as well as possible. Maybe even by using interval analysis as you suggest (which is also what Conal did to support predicate
if I recall correctly). But, the first implementation step to get basic support is to implement pull streams.
What do you want to happen if you do
time.log()
?
As long as this doesn't crash peoples' computers, I want:
1551092289
1551092290
1551092291
1551092292
1551092293
1551092294
1551092295
1551092296
1551092297
If you want it to log less, divide by a multiple of 10 and round it so that it changes less often.
To me that doesn't sound very useful. That would log 1000 messages per second which would drown everything else. The key purpose of .log()
is to do what is most useful for debugging purposes and having a configurable limit that defaults to a sensible value sounds most useful to me.
We may be optimizing for different perspectives. I use .log()
more for "is this flow working at all / producing any data?", "how often is this happening?", and "what's the general shape of this flow's data?" and less for fine-grained analysis.
I think I'd be quite confused if I did .log()
on time and it defaulted to logging once per second. The 1000 messages per second very accurately communicates how time
behaves as a very-often changing entity. Of course it would drown everything else out, but that's not a problem because I'm just looking to see what it looks like and now that I see that, I'll delete that line.
Ultimately, I'd like a uniform interface and having to specify a sample rate that's only applied for pull-based behaviors feels jagged to me. Would we do the same for changes()
? What if I do changes(time).log()
?
I think I'd be quite confused if I did .log() on time and it defaulted to logging once per second. The 1000 messages per second very accurately communicates how time behaves as a very-often changing entity. Of course it would drown everything else out, but that's not a problem because I'm just looking to see what it looks like and now that I see that, I'll delete that line.
That makes a lot of sense and I can definitely see the point here. If I understand you correctly, then from this perspective time.log()
logging once per second would actually be misleading because it would give the wrong impression that time
only changes once per second.
Ultimately, I'd like a uniform interface and having to specify a sample rate that's only applied for pull-based behaviors feels jagged to me. Would we do the same for
changes()
? What if I dochanges(time).log()
?
From my perspective log
and changes
exists on two different levels in the library. In this thread we are lumping them together but I'd just emphasize that I think they're very different. I see changes
as a proper function in the FRP library that should have semantics and be well behaved. But, I only see log
as a handy utility function for debugging purposes. I definitely agree that having to specify a sample rate feels jagged. To me that's acceptable for log
as it increases it value as a handy utility function for debugging. But, I would not at all want to do the same thing for changes
.
So, I agree that specifying a sample rate is not pretty. But, I think log
falls outside the group of functions that should be pretty. I use it for quick and dirty logging. It's also a function that sits at the "edge" of the FRP library since it's used to execute side-effects (logging) and not a pure function that manipulates FRP primitives. For these "edge"-functions we cannot completely hide the difference between push and pull behaviors. For this reason the log
function has to apply some sampling interval. We can make it very low to catch all or close to all changes?
It sounds like we are mostly on the same page. My only remaining confusing is that to me it seems like our difficulty with log
reduces to our difficulty with changes
. If you can have a well-behaved changes
that does not expose the underlying push-or-pull-strategy, why don't we use that same interface in log
?
Put another way: we could disallow log
on behaviors because that's a strange notion anyway. If you want to log
a behavior, first apply changes
. While this is slightly more verbose, I think it'd be worthwhile to keep the interface clean.
Another alternative is build a log
function that automatically applies changes
to behaviors internally. I wouldn't protest to this, but I do prefer a library with less magic and more explicitness.
So my question to you: why would log
not simply use a well-behaved changes
for behaviors? In other words, log
could be a performStream of console.log
mapped to changes of a behavior.
we could disallow log on behaviors because that's a strange notion anyway.
Why is that a strange notion?
why would log not simply use a well-behaved changes for behaviors? In other words, log could be a performStream of console.log mapped to changes of a behavior.
From my point of view, because that conflicts with what I think is the purpose of log
:wink:. I think it should be a handy utility function for debugging purposes. Getting ones console drowned in a rapid rate of messages is not necessarily very useful for debugging.
I don't normally use console.log for continuous values, because as you say, a ton of quick messages isn't that helpful. Often I whip up an on-screen debugging interface that will just show the current value at any given time. But I don't think we can do that in the console. It's only for static values.
I don't normally use console.log for continuous values, because as you say, a ton of quick messages isn't that helpful.
Yeah, the optional interval can hopefully help a bit with this.
Often I whip up an on-screen debugging interface that will just show the current value at any given time. But I don't think we can do that in the console. It's only for static values.
That is a really nice solution and definitely a nicer experience.
I've made attempts to close this issue:
changes
fail loudly with an exception when given a pull behavior.log
work on pull behavior. Per default, it logs with a high resolution of 10 logs per second but this can be changed through an optional argument.What do you think @stevekrouse?
Well done! Appreciate all this discussion and for getting this fixed
Me too. I realised that I didn't answer the following:
It feels like pull-based behaviors a really different kind of thing here. Would it be crazy to have a separate type for them?
I've thought about that as well. I don't think it would be crazy. But, I think there are good reasons for doing what we do now.
First of all the difference between pull and push behaviors is an implementation detail. Conceptually or semantically the push/pull distinction doesn't exist. Hence we should try to hide that difference as much as possible. And having a PushBehavior
and a PullBehavior
does the opposite of that.
The distinction that one actually could make is between behaviors that change every now and then (i.e. behaviors that change discretly many times or where the numbers of changes are countable) and behaviors that change infinitely often. This distinction makes sense conceptually—without thinking about implementation. Furthermore, calling change
on the former works, but calling changes
on the latter does not. So keeping those two things separate could be beneficial. However, even if we wanted to make that distinction in the types I don't think we could. It seems impossible. We can't detect that time.map(t => Math.floor(t / 100))
changes discretely often but that time.map(t => t * t)
changes infinitely often.
I think what we have right now can be compared to infinite lists in Haskell. Behaviors that changes infinitely often and lists of infinite length both contain infinite information (I've heard Conal make the same analogy). There are many things that one can do with this infinite information that works fine. But, some things, like calling changes
on behavior that changes infinitely often or trying to take the length of an infinite list, will "explode". And this cannot be prevented without also preventing some very desirable things.
Great analysis. Thank you!
I was reading push-pull FRP and it seems like reactive behaviors are Conal's phrase for PushBehaviors, or behaviors that change discretely. He does some clever stuff to figure out which parts of behaviors are discrete and which are continuous, but I don't think those would transfer over to a JavaScript framework.
It should at least error if it's not going to show you anything, but I might prefer it to show me all the milliseconds.