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

Broadcast for diagonal should be lazy #48871

Open
putianyi889 opened this issue Mar 2, 2023 · 5 comments
Open

Broadcast for diagonal should be lazy #48871

putianyi889 opened this issue Mar 2, 2023 · 5 comments
Labels
broadcast Applying a function over a collection

Comments

@putianyi889
Copy link
Contributor

Similar issue #48443
Many other functions that should be equivalent to their broadcasted version have the same problem (although some of them don't even lazy broadcast to ranges). Here is an example.

julia> Diagonal(1:5)+Diagonal(1:5) |> typeof
Diagonal{Int64, StepRangeLen{Int64, Int64, Int64, Int64}}

julia> Diagonal(1:5).+Diagonal(1:5) |> typeof
Diagonal{Int64, Vector{Int64}}

julia> (1:5) .+ (1:5) |> typeof
StepRangeLen{Int64, Int64, Int64, Int64}
@putianyi889
Copy link
Contributor Author

And here are some other inconsistencies that might not relate to Diagonal

julia> float(Diagonal(1:5)) |> typeof
Diagonal{Float64, Vector{Float64}}

julia> float.(Diagonal(1:5)) |> typeof
Diagonal{Float64, Vector{Float64}}

julia> float(1:5) |> typeof
StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}

julia> float.(1:5) |> typeof
Vector{Float64} (alias for Array{Float64, 1})

julia> Float64.(1:5) |> typeof
Vector{Float64} (alias for Array{Float64, 1})
julia> conj(Diagonal(1:5)) |> typeof
Diagonal{Int64, UnitRange{Int64}}

julia> conj.(Diagonal(1:5)) |> typeof
Diagonal{Int64, Vector{Int64}}

julia> conj(1:5) |> typeof
UnitRange{Int64}

julia> conj.(1:5) |> typeof
Vector{Int64} (alias for Array{Int64, 1})

@jishnub
Copy link
Contributor

jishnub commented Mar 3, 2023

This doesn't seem like a problem with the operation being lazy, rather than the type being materialized as a vector rather than being retained as a range. If anything, the operation is not as eager as it should be while broadcasting.

If I understand correctly, the result is correct, but it might be possible to reduce the allocations.

@putianyi889
Copy link
Contributor Author

The problem is that the results don't follow the same rule. If broadcasting materializes, it should also materialize other types, so that (1:5).+(1:5) is a Vector. If conj(Diagonal(v))=Diagonal(conj(v)), then float(Diagonal(v)) should be Diagonal(float(v)).

@putianyi889
Copy link
Contributor Author

A consequence to that is that I don't know what the type of the result is unless I experiment. Following that, they increase the work needed to develop other related types.

@jishnub
Copy link
Contributor

jishnub commented Mar 3, 2023

Certain broadcast operations are special-cased for ranges, but not for wrappers of ranges. I see what you mean, but I don't think there's any guarantee as such regarding the type of the result, and the special cases are treated as exceptions, rather than the rule. I agree that it would be good to be consistent, but perhaps a general approach needs to be trait-based (as in #48861), rather than the special cases that we have at the moment.

@nsajko nsajko added the broadcast Applying a function over a collection label Jul 15, 2024
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

3 participants