-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Comments
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}) |
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. |
The problem is that the results don't follow the same rule. If broadcasting materializes, it should also materialize other types, so that |
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. |
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. |
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.
The text was updated successfully, but these errors were encountered: