Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Apr 29, 2021

In v1.6

julia> using OffsetArrays

julia> ones(10)[Base.IdentityUnitRange(2:3)]
2-element OffsetArray(::Vector{Float64}, 2:3) with eltype Float64 with indices 2:3:
 1.0
 1.0

On master:

julia> ones(10)[Base.IdentityUnitRange(2:3)]
2-element Vector{Float64}:
 1.0
 1.0

This happens because #39896 changed the behavior of getindex, and it now uses the length of the indices instead of the axes to generate the output array. This PR reverts this to make the result account for the axes again. After this:

julia> ones(10)[Base.IdentityUnitRange(2:3)]
2-element OffsetArray(::Vector{Float64}, 2:3) with eltype Float64 with indices 2:3:
 1.0
 1.0

Also,

on master

julia> using StaticArrays

julia> ones(10)[SOneTo(2)]
2-element Vector{Float64}:
 1.0
 1.0

After this PR (back to v1.6 behavior):

julia> ones(10)[SOneTo(2)]
2-element SizedVector{2, Float64, Vector{Float64}} with indices SOneTo(2):
 1.0
 1.0

The performance impact on changing unsafe_copyto! to copyto! appears to be minimal.

with copyto!

julia> a = ones(20000);

julia> @btime $a[$ax];
  19.084 μs (2 allocations: 156.33 KiB)

with unsafe_copyto!

julia> @btime $a[$ax];
  18.968 μs (2 allocations: 156.33 KiB)

The difference is within the noise level.

@simeonschaub simeonschaub added arrays [a, r, r, a, y, s] bugfix This change fixes an existing bug labels Apr 29, 2021
@simeonschaub simeonschaub merged commit 09d6a03 into JuliaLang:master Apr 29, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
jarlebring pushed a commit to jarlebring/julia that referenced this pull request May 4, 2021
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays [a, r, r, a, y, s] bugfix This change fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants