-
Notifications
You must be signed in to change notification settings - Fork 28
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
Lazy broadcasting bug? #38
Comments
This is because julia> LazyArray(@~ 1 .* 2)
ERROR: MethodError: no method matching BroadcastArray{Int64,N,BRD} where BRD<:Broadcasted where N(::Broadcasted{Base.Broadcast.DefaultArrayStyle{0},Nothing,typeof(*),Tuple{Int64,Int64}})
Closest candidates are:
BroadcastArray{Int64,N,BRD} where BRD<:Broadcasted where N(::Broadcasted{#s16,#s15,#s14,#s13} where #s13<:Tuple where #s14 where #s15<:Tuple{Vararg{Any,N}} where #s16<:Union{Nothing, BroadcastStyle}) where {T, N} at /home/takafumi/.julia/dev/LazyArrays/src/lazybroadcasting.jl:13
Stacktrace:
[1] _BroadcastArray(::Broadcasted{Base.Broadcast.DefaultArrayStyle{0},Nothing,typeof(*),Tuple{Int64,Int64}}) at /home/takafumi/.julia/dev/LazyArrays/src/lazybroadcasting.jl:19
[2] BroadcastArray(::Broadcasted{Base.Broadcast.DefaultArrayStyle{0},Nothing,typeof(*),Tuple{Int64,Int64}}) at /home/takafumi/.julia/dev/LazyArrays/src/lazybroadcasting.jl:20
[3] LazyArray(::Broadcasted{Base.Broadcast.DefaultArrayStyle{0},Nothing,typeof(*),Tuple{Int64,Int64}}) at /home/takafumi/.julia/dev/LazyArrays/src/lazybroadcasting.jl:10
[4] top-level scope at none:0
julia> typeof(@~ 1 .* 2)
Broadcasted{Base.Broadcast.DefaultArrayStyle{0},Nothing,typeof(*),Tuple{Int64,Int64}}
LazyArrays.jl/src/lazybroadcasting.jl Lines 13 to 15 in 3b6be06
Adding the following definition fixes this issue: BroadcastArray{T}(bc::Broadcasted{<:Union{Nothing,BroadcastStyle},Nothing,<:Any,<:Tuple}) where {T} =
BroadcastArray{T,0}(bc) But I wonder why |
I guess what one would one from a lazy scalar is to somehow preserve constant propagation? |
Thanks, that's much simpler than my example! The suggested change means that It's possible that you should instead just not be lazy about scalar + scalar broadcasting at all. Are there examples where scalar laziness would be desirable? It would seem strange for the lazy version to return a container when the ordinary one does not. |
FYI: I just dug into the code a bit more. The special handling does seem to be intentional. It's explicitly defined that instantiate(bc::Broadcasted{<:Union{AbstractArrayStyle{0}, Style{Tuple}}}) = bc in https://github.com/JuliaLang/julia/blob/48634f9f8669e1dc1be0a1589cd5be880c04055a/base/broadcast.jl#L263 which committed in JuliaLang/julia#26891; i.e., it was there from the very beginning. But I'm not sure what it means... @dlfivefifty I don't quite get what you are saying. But I guess you mean it was some kind of performance optimization? So it's safe to handle scalars as 0-dim array?
It's a bit contrived example, but
julia> copy(@~ 1 .* 2) :: Int
2
julia> Broadcast.materialize(@~ 1 .* 2) :: Int
2 I think it's OK for |
I just ran into the following error:
The same happens for
@~ @. (2s) * R
obviously.This is for LazyArrays v0.8.1, Julia 1.1. On master, #31 has changed things such that I get the error only when calling
LazyArray
:The text was updated successfully, but these errors were encountered: