Defined pointer method.

[chriselrod]

This is needed for transitioning LoopVectorization to being based off ArrayInterface.

[codecov]

Codecov Report

Merging #170 (6dcded3) into master (f20ecfc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #170   +/-   ##
=======================================
  Coverage   99.22%   99.23%           
=======================================
  Files           4        4           
  Lines         257      260    +3     
=======================================
+ Hits          255      258    +3     
  Misses          2        2           

Impacted Files	Coverage Δ
src/OffsetArrays.jl	99.45% <100.00%> (+<0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f20ecfc...6dcded3. Read the comment docs.

[chriselrod]

The unit test fails on Julia 1.0. LoopVectorization doesn't support 1.0, so I don't need that test to pass...
The failure is because pointer wasn't defined for Adjoint wrappers in 1.0.

I'll try just checking version, and only checking pointer(::Adjoint{T,<:OffsetArray}) on 1.5 and newer.

[chriselrod]

@jishnub Mind reviewing?

[timholy]

Should this call unsafe_convert instead of pointer? I'm not really clear on when one vs the other should be used.

[chriselrod]

I'm not either. But after pondering for a couple minutes, I think unsafe_convert would be a little better so that we can specify that we want a Ptr{T}.

The advantage of pointer would be that it falls back on unsafe_convert, giving another option for authors to intercept. I don't really know why anyone would want pointer or unsafe_convert to return a different value. Maybe a different type, in which case we want to use unsafe_convert anyway.

[jishnub]

@jishnub Mind reviewing?

I'm afraid I don't know much about this, perhaps @timholy would be the right person to review

[timholy]

Let's do this. We might want to rename index->offset in the docstring for pointer(array, index), where offset is a 1-based displacement from the start of the array. Do you want to do that or should I?

[chriselrod]

Great, sounds good.
I could make the PR.
The doc string also mentions Ref:

Calling [`Ref(array[, index])`](@ref Ref) is generally preferable to this function as it guarantees validity.

Refs do use the index:

julia> v = rand(100);

julia> vo = OffsetArray(v, -10);

julia> v[1]
0.9243160384144822

julia> v[11]
0.7486822152275043

julia> Ref(vo, 1)[]
0.7486822152275043

I'm not sure if a docstring like this:

"""
    pointer(array [, offset])

Get the native address of an array or string, optionally at a given location `offset`.

This function is "unsafe". Be careful to ensure that a Julia reference to
`array` exists as long as this pointer will be used. The [`GC.@preserve`](@ref)
macro should be used to protect the `array` argument from garbage collection
within a given block of code.

Calling [`Ref(array[, index])`](@ref Ref) is generally preferable to this function as it guarantees validity.
"""

would be confusing without explaining index = offset + firstindex(array) - 1?

Also, perhaps we should add that offset = 1 corresponds to no offset? From the name offset, readers may otherwise assume that offset is zero-based rather than 1-based.

[timholy]

I guess the other option would be to keep it an index and return a pointer that is shifted so that pointer(a, firstindex(a)) returns the pointer to the first item.

[chriselrod]

Yeah, that'd probably be better.
I'll change ArrayInterface/VectorizationBase/LoopVectorization to match that behavior.

[timholy]

You probably already know this, but just in case you're not familiar with the differences in linear indexing for 1-d arrays vs all others: https://docs.julialang.org/en/v1/devdocs/offset-arrays/#Linear-indexing-(LinearIndices).

[chriselrod]

Actually turns out the default definition for Base.pointer(array::AbtractArray, index::Integer) already produces the correct behavior (i.e., that index really is the index, not the offset) thanks to the base fallback:

function pointer(x::AbstractArray{T}, i::Integer) where T
    @_inline_meta
    unsafe_convert(Ptr{T}, x) + Int(_memory_offset(x, i))::Int
end

# The distance from pointer(x) to the element at x[I...] in bytes
function _memory_offset(x::AbstractArray, I::Vararg{Any,N}) where {N}
    J = _to_subscript_indices(x, I...)
    return sum(map((i, s, o)->s*(i-o), J, strides(x), Tuple(first(CartesianIndices(x)))))*elsize(x)
end

once we define:

Base.strides(A::OffsetArray) = strides(parent(A))
Base.elsize(::Type{OffsetArray{T,N,A}}) where {T,N,A} = Base.elsize(A)

That means:

julia> using OffsetArrays, Test

julia> a = OffsetVector(collect(10:20), 9);

julia> @test 12 == a[12] == unsafe_load(pointer(a), 12 + (1 - firstindex(a))) == unsafe_load(pointer(a, 12))
Test Passed

julia> A = OffsetArray(reshape(collect(10:130), (11,11)), 9, 9);

julia> @test 21 == A[12] == unsafe_load(pointer(A), 12) == unsafe_load(pointer(A, 12))
Test Passed

julia> @test 61 == A[52] == unsafe_load(pointer(A), 52) == unsafe_load(pointer(A, 52))
Test Passed

I added those methods and a few more tests.

You probably already know this, but just in case you're not familiar with the differences in linear indexing for 1-d arrays vs all others:

Thanks, I observed and worked around that behavior in OffsetArrays, but good to know that it's a general rule I can rely upon. (Obviously someone could define types breaking any rule, but that'd put the onus on them / they'd be needing to overwrite a huge number of other methods anyway to get their desired behavior).

[timholy]

Very nice! Merge at will.

[chriselrod]

Shall we tag a release?

[timholy]

Done. I'm never sure whether to call these patch releases or minor releases (is this a new feature or a bug fix?), but I went with the version bump you supplied.