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

Generalize f.(A) to tuples? #17411

Closed
timholy opened this issue Jul 14, 2016 · 16 comments
Closed

Generalize f.(A) to tuples? #17411

timholy opened this issue Jul 14, 2016 · 16 comments
Labels
broadcast Applying a function over a collection

Comments

@timholy
Copy link
Member

timholy commented Jul 14, 2016

f.(A) is shorthand for broadcast(f, A), but also equivalent to map(f, A). Since we support map for both arrays and tuples, should we also support f.(t) for tuples? I may be missing it, but I can't find any place this has been discussed previously (e.g., #15032).

julia> abs.([2, -5])
2-element Array{Int64,1}:
 2
 5

julia> abs.((2, -5))
ERROR: invalid Array dimensions
 in Array{Union{},N}(::Tuple{Int64,Int64}) at ./boot.jl:319
 in similar(::Type{T}, ::Tuple{Int64,Int64}) at ./abstractarray.jl:407
 in broadcast_t(::Function, ::Type{Any}, ::Tuple{Int64,Int64}, ::Vararg{Tuple{Int64,Int64},N}) at ./broadcast.jl:189
 in broadcast(::Function, ::Tuple{Int64,Int64}) at ./broadcast.jl:204
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46
@pabloferz
Copy link
Contributor

Related #16986

@pabloferz
Copy link
Contributor

pabloferz commented Jul 14, 2016

Actually I have just a couple of lines over that PR that should cover the most relevant cases

@inline broadcast(f, a::Tuple) = map(f, a)
@inline broadcast{N}(f, a::NTuple{N}, bs::Vararg{NTuple{N}}) = map(f, a, bs...)

@martinholters
Copy link
Member

Supporting tuples sounds like a good idea at first glance, but before rushing into things, we should consider what we want to do with mixed cases. E.g. broadcast(+, (1,2), 1) == (2,3)? Sounds like a plausible extension. Mixing tuples and arrays (treating tuples like vectors)? Would be the logical next step, but I'm not convinced we really want that.

@stevengj
Copy link
Member

My first instinct would be to just treat a tuple as a 1d array, and produce array results. Anything else would be hard to generalize to mixed arguments.

@pabloferz
Copy link
Contributor

What I had in mind for broadcast was the following

  • If all the arguments are scalars, return a scalar.
  • If the arguments are tuples and zero or more scalars, return a tuple.
  • If there is at least an array return an array (and treat tuples as 1d arrays).

@StefanKarpinski
Copy link
Member

I was about to object that we don't have n-dimensional tuples, but then I realized that's not actually a problem since tuples are only 1-dimensional and if they're broadcastable as 1-d objects, then they must all be the same length. So @pabloferz's proposal seems sane to me.

@stevengj
Copy link
Member

Let me make an alternative suggestion: if the arguments are all tuples, then we fall through to map, which produces a tuple. Otherwise (including mixed tuples and scalars), we use the ordinary broadcast machinery, which produces an Array.

(Among other things, this will greatly simplify the implementation — I'd hate to have to write a specialized broadcast function just for tuples & scalars.)

@eschnett
Copy link
Contributor

When I use tuples, I usually want to write code that is fast in the sense that no memory is allocated; data can be kept in registers or on the stack. If none of the input types is an array, then I would find it annoying if the result was an array.

If the inputs consisted of sparse arrays and scalars, what should the result be? A dense array, or a sparse array? What if the input consisted of distributed arrays and scalars? Should all the combinations "X + scalar" lead to a dense array? It would certainly simplify the implementation, but would it be useful?

@timholy
Copy link
Member Author

timholy commented Jul 27, 2016

If you're doing anything "interesting" with tuples, usually you want pretty specialized behavior: in the array code, for example, there are many cases where you need customized rules for what to do when one tuple is shorter than the other, or when you are "consuming" two tuples but the rules for advancing on to the next argument depend on the types of the first items in each tuple. I doubt that any generic function could replace very much of that logic.

So I vote for keeping it dirt simple: insist that all inputs are tuples (and fall through to map), and in any other case just let the MethodError happen.

@pabloferz
Copy link
Contributor

The current proposal over #16986 handles thing as per my #17411 (comment) above, in case someone wants to help review it.

@yurivish
Copy link
Contributor

yurivish commented Jul 27, 2016

I also wonder whether f.(generator expression) should work. For example, you can do map(identity, (n for n in 1:5)) but identity.(n for n in 1:5) currently fails with

RROR: MethodError: no method matching indices(::Base.Generator{UnitRange{Int64},##11#12})
Closest candidates are:
  indices(::SimpleVector)
  indices(::SimpleVector, ::Any)
  indices(::Number)
  ...
 in broadcast_shape(::Base.Generator{UnitRange{Int64},##11#12}) at ./broadcast.jl:21
 in broadcast_t(::Function, ::Type{Any}, ::Base.Generator{UnitRange{Int64},##11#12}, ::Vararg{Base.Generator{UnitRange{Int64},##11#12},N}) at ./broadcast.jl:204
 in broadcast(::Function, ::Base.Generator{UnitRange{Int64},##11#12}) at ./broadcast.jl:221
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

I couldn't find a good description of the meaning difference of broadcast vs. map, so maybe the answer here is that it should error.

@stevengj
Copy link
Member

stevengj commented Jul 27, 2016

At least Generator{T<:AbstractArray} seems like it should workable. (But we'll need to implement getindex for that case etc, so maybe that should be a separate issue @yurivish.)

@toivoh
Copy link
Contributor

toivoh commented Jul 31, 2016

One use case for tuples with broadcast could be to protect an argument from broadcasting that you want to treat as a scalar instead of an indexable. It would be good if this case could be supported (or we create another wrapper type for this purpose, that avoids the overhead of allocating an Array).

@stevengj stevengj added the broadcast Applying a function over a collection label Aug 2, 2016
@stevengj
Copy link
Member

Should be closed by #16986

@cossio
Copy link
Contributor

cossio commented Aug 18, 2017

@stevengj Why was this closed?

The following example does not work:

x = y = 0.
x, y .+= 2.

Doesn't it fall under this umbrella?

@Sacha0
Copy link
Member

Sacha0 commented Aug 18, 2017

The following example does not work

x, y .+= z presently lowers to broadcast!(+, (x, y), (x, y), z), which fails as no such method exists (tuples being immutable).

Doesn't it fall under this umbrella?

Not precisely. Please file a separate issue. Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection
Projects
None yet
Development

No branches or pull requests

10 participants