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

Indexing arrays with AbstractUnitRanges retains indices #40660

Merged
merged 1 commit into from
Apr 29, 2021

Conversation

jishnub
Copy link
Contributor

@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