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

Zero-dimensional views of AbstractVectors assume that the parent is 1-indexed #37274

Closed
jishnub opened this issue Aug 29, 2020 · 4 comments · Fixed by #39404
Closed

Zero-dimensional views of AbstractVectors assume that the parent is 1-indexed #37274

jishnub opened this issue Aug 29, 2020 · 4 comments · Fixed by #39404
Labels
arrays [a, r, r, a, y, s] bug Indicates an unexpected problem or unintended behavior

Comments

@jishnub
Copy link
Contributor

jishnub commented Aug 29, 2020

This is the same issue reported in OffsetArrays, but I'm posting it here as I'm not sure if the fix should go into Base. Please close this if it's more appropriate for the fix to be in OffsetArrays.

For a zero-dimensional array a we usually expect a[] == a[1]. However this does not work with zero-D views of OffsetVectors

julia> using OffsetArrays

julia> a = OffsetArray(1:3, 0:2);

julia> b = @view a[0]
0-dimensional view(OffsetArray(::UnitRange{Int64}, 0:2), 0) with eltype Int64:
1

julia> b[]
1

julia> b[1]
2
@stevengj
Copy link
Member

I've traced it down to this line, which seems like it incorrectly assumes that linear indices start at 1.

@timholy, it seems that you wrote that code back in #16260, can you comment?

@stevengj stevengj added arrays [a, r, r, a, y, s] bug Indicates an unexpected problem or unintended behavior labels Aug 31, 2020
@timholy
Copy link
Member

timholy commented Aug 31, 2020

@timholy, it seems that you wrote that code back in #16260, can you comment?

I often struggle to remember last Tuesday, so I don't recall anything useful, but that looks like it predates offset axes (going by the blog post date). Thanks for catching this use case, @jishnub.

@stevengj, that line enacts the "linear indexing starts at 1" policy with the exemption for 1d arrays on

julia/base/subarray.jl

Lines 377 to 378 in 41e603e

compute_offset1(parent, stride1::Integer, dims::Tuple{Int}, inds::Tuple{Slice}, I::Tuple) =
(@_inline_meta; compute_linindex(parent, I) - stride1*first(axes(parent, dims[1]))) # index-preserving case

I'm guessing we need a 0-dimensional exemption as well?

@stevengj
Copy link
Member

I'm guessing we need a 0-dimensional exemption as well?

The underlying array is 1d in this case, and since this is computing an offset index into the original array shouldn't the 1d exemption apply?

@fghorow
Copy link

fghorow commented Oct 13, 2020

A question: would this bug affect the ordering of an OffsetArray (I_Ωpp) being written out via a Numpy call of the form np.save("./I_Omega_pp_R_interpolated.npy",I_Ωpp)?
If so, I've been chasing problems caused by this for weeks! :-(

Ooops. Edited to add: using PyCall; np = pyimport("numpy") prior to referencing np...

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] bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants