Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

time.log() fails silently #91

Closed
stevekrouse opened this issue Dec 12, 2018 · 19 comments
Closed

time.log() fails silently #91

stevekrouse opened this issue Dec 12, 2018 · 19 comments

Comments

@stevekrouse
Copy link
Contributor

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.

@paldepind
Copy link
Member

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.

@stevekrouse
Copy link
Contributor Author

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?

@stevekrouse
Copy link
Contributor Author

This issue now also tracks the fact that changes(time) also fails silently

@stevekrouse
Copy link
Contributor Author

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.

@paldepind
Copy link
Member

paldepind commented Feb 23, 2019

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.

image

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).

@stevekrouse
Copy link
Contributor Author

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?

@paldepind
Copy link
Member

paldepind commented Feb 25, 2019

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() or changes(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 😄 Hareactive should support that to the extend possible. We have an issue open about that: funkia/hareactive#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.

@stevekrouse
Copy link
Contributor Author

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.

@paldepind
Copy link
Member

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.

@stevekrouse
Copy link
Contributor Author

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()?

@paldepind
Copy link
Member

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 do changes(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?

@stevekrouse
Copy link
Contributor Author

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.

@paldepind
Copy link
Member

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 😉. 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.

@stevekrouse
Copy link
Contributor Author

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.

@paldepind
Copy link
Member

paldepind commented Apr 12, 2019

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:

  • funkia/hareactive@3cb9915 makes changes fail loudly with an exception when given a pull behavior.
  • funkia/hareactive@6696f1d makes 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?

@stevekrouse
Copy link
Contributor Author

Well done! Appreciate all this discussion and for getting this fixed

@paldepind
Copy link
Member

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.

@stevekrouse
Copy link
Contributor Author

Great analysis. Thank you!

@stevekrouse
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants