-
Notifications
You must be signed in to change notification settings - Fork 46
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
RFC: A simpler implementation #65
Conversation
This is a simpler implementation of Reactive. - A signal Node contains the value and a vector of weak refs of functions that need to be run when the node updates - `connect_*` functions (e.g. consume_connect) take both input and output nodes and add appropriate actions to them - exported functions like `consume`, `foldp`, etc create new nodes and connect them to the input nodes using the respective `connect_*` function - `_messages` is a global channel. `push!` writes `(node, new_value, metadata)` to the channel. there is _recipients_dict which is a weak key dict of nodes to a list of action functions *that depend on it* - `Reactive.run(n)` reads n messages off this channel and propagates the changes. It throws a `ReactiveException` with data to debug the crash. - no `@lift` macro - it is too complex as it stands This is a breaking change - Signal and Input are both typealiases to Node. `Input{Type}(val)` is now `Input(Type, val)` - `push!` takes optional debug metadata argument which shows up in the `ReactiveException` object if an exception occurs - `merge` breaks the guarantee of keeping the update from the earliest argument if more than one arguments update in the same timestep - the oldest node gets priority (the propagation is depth-first) - the main process needs to run `Reactive.run()` and watch for exceptions. - added `delay`, `debounce` and `boundcopy` - setting `node.alive` to false stops updates from propagating from a node You can call `push!` inside a `consume` this simply queues up the update for the next timestep.
iter += 1 | ||
|
||
waiting = true | ||
(node, value, debug_meta) = take!(_messages) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you could address #62 by doing an isready
loop and setting the value
of all the nodes at once while queuing the actions in a Set
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating many inputs at once may change some semantics. E.g. the value of foldp(x -> x+1, 0, map(tuple, signal1, signal2))
will not be the same for the same sequences of input to signal1 and signal2 depending on how fast the runner is able to clear the queue... It also affects sampleon
, filterwhen
in subtle ways as well (but maybe these two are not as important as foldp)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You understand the complexities much better than me. I'll simply add a use-case to #62.
I'm pretty new to Reactive and haven't yet used it in a serious project, but a big 👍 for this. I'm especially excited to see the removal of all of those heap operations and like the While you're renaming things, I'll confess that the |
@timholy great to hear! :) I agree |
Would be good to also have the names reflect the "imperative"/"declarative" distinction clearly. Wow, naming is hard. |
Or maybe |
why a |
I was thinking that |
(Almost like |
Ah, I see the similarity with |
We seem to have correct implementations of everything we need now. Couple of questions to answer to get this merged:
|
1.) |
|
Congratulations!
I am torn on this one. On balance, I suspect that it would be better that way. But I do have a question: suppose (like a = foo(7) but internally in the course of creating
Yes, definitely good to reduce the number of redundant names. You know about I agree with the preference |
I have another implementation of preserve which will allow
I didn't know about it, will use it. Thanks!
|
Here's the preserve we need. |
I agree that |
Conflicts: .travis.yml REQUIRE src/Reactive.jl src/timing.jl test/basics.jl test/flatten.jl test/macro.jl test/trylift.jl
it prints Reactive.Node instead of Reactive.Signal This reverts commit acffeb1.
Awesome! Thanks for the great work :) |
Yes indeed! |
This is a rewrite of Reactive to make it more robust
How it works
Node
contains the current value and a vector of Actionsadd_action!(f, node, recipient_node)
creates an Action withweakref torecipient_node and function f.send_value!(node, val, timestep)
sets val as node's value and calls all the actions with therecipient_node
andtimestep
as the two arguments tof
.timestep
is a counter that increases with every new message.connect_*
functions (e.g.connect_map
) take both input and output nodes and add appropriate actions to them (spot the difference with this example in sicp)map
,foldp
, etc create a new output node and connect them to input nodes using the respectiveconnect_*
function_messages
is a global channel.push!
writes(node, new_value, metadata)
to the channel.Reactive.run(n)
reads n messages off this channel and propagates the changes.It throws a
ReactiveException
with data to debug the crash. aReactiveException
prints a comprehensive crash message, for example:@lift
macro - it is too complex as it standsAPI changes
This is a breaking change
Input{Type}(val)
is nowInput(Type, val)
lift
is nowmap
foldl
is nowfoldp
- for fold-over-pastmerge
breaks the guarantee of keeping the update from the earliestargument if more than one arguments update in the same timestep - the
oldest node gets priority (the propagation is depth-first)
Reactive.run()
and watch for exceptions. AReactiveException
is raised which captures exceptions, backtraces and more info. exceptions don't get lost into the etherpush!
takes optionalmeta
argument which shows up in theReactiveException
object if an exception occursclose(node)
makes the node refuse further updatesdelay
,debounce
andboundcopy
You can call
push!
inside aconsume
this simply queues up the update for the next timestep.This fixes #54 #62 #61 #24
TODO
right now the automatic gc'ing of nodes does not work: make it work with at least with some explicit api (relevant code is here)cc @SimonDanisch @Keno @stevengj @aviks @amitmurthy