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

lift API tweaks #2

Closed
stevengj opened this issue Jun 10, 2014 · 4 comments
Closed

lift API tweaks #2

stevengj opened this issue Jun 10, 2014 · 4 comments

Comments

@stevengj
Copy link
Contributor

I would suggest defining it as:

lift(f::Function, output_type::Type, inputs::Signal...)

so that:

  • Putting the Function argument first allows one to use do-block syntax with lift.
  • Using Type rather than DataType allows functions that return tuples of types.

It would eventually be nice to have:

valtype{T}(::Signal{T}) = T
lift(f::Function, inputs::Signal...) = lift(f, return_type(f, tuple(map(valtype, inputs))), inputs...)

so that you don't need to specify the return type if Julia can infer it. The return_type functionality is not in Julia yet, though; see JuliaLang/julia#6692

@stevengj
Copy link
Contributor Author

Also, it seems like it would be much cleaner, and probably better-performing (JuliaLang/julia#1864), just to have lift return an instance of a Lift <: Signal type that stores f and the signals.

Similarly for filter etc. Why not get rid of all the recv functions and replace them with an update(s::Signal, parent::Signal) method that is overloaded as needed for each s::Signal type?

@stevengj
Copy link
Contributor Author

The count == n check seems odd to me. Even if I only push! a new value to one of the lift inputs, surely the lifted signal should get updated.

@stevengj stevengj mentioned this issue Jun 10, 2014
@shashi shashi closed this as completed in a6bd393 Jun 12, 2014
@stevengj
Copy link
Contributor Author

It still isn't quite ideal. The Lift definition should really be something like:

type Lift{T} <: Node{T}
    rank :: Uint
    children :: Vector{Signal}
    f :: Function
    signals :: (Signal...)
    value :: T
    function Lift(f :: Function, signals :: Signal...)
        node = new(next_rank(), Signal[], f, signals, convert(T, f([s.value for s in signals]...)))
        add_child!(signals, node)
        return node
    end
end

function update{T, U}(node :: Lift{T}, parent :: Signal{U})
    node.value = convert(T, node.f([s.value for s in node.signals]...))
    return true
end

Note that I avoid the double indirection of having update call apply_f which calls f: it can just call f directly. Note also that I convert f's result to T rather than asserting it is of type T.

@shashi
Copy link
Member

shashi commented Jun 12, 2014

Right. Fixed this.

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