Int8 Offsets

[simone-silvestri]

Hello, first of all, thanks for the beautiful package. Over at Oceananigans.jl and ClimaOcean.jl, we use offset arrays to store the underlying data of our Field types. However, now that we are increasing the complexity of the model, we are having increasingly frequent problems with GPU parameter space when launching kernels that require many offset array inputs.

We think that this has to do with the fact that offsets are Int64 types, which consume quite a lot of parameter space, however, we typically need offsets in the range of -10 .. 10, so Int8 would be enough for our scope.

However, it looks like the Int type is hardcoded in the OffsetArray type.

OffsetArrays.jl/src/OffsetArrays.jl

Line 114 in 5972e9e

offsets::NTuple{N,Int}

Is there a specific reason for having offsets as Int64s or would it be possible to allow flexibility in the sizes of the offsets?

In the latter case, could I attempt a PR allowing lower-sized offsets?

cc @glwagner

[simone-silvestri]

Wondering if it was possible to find a solution for this issue. Cheers.

[jishnub]

A PR is welcome

[simone-silvestri]

thanks @jishnub, I ll give it a try

[jishnub]

I think the choice of Int offsets is motivated by the fact that axes of AbstractArrays typically are Int ranges. The PR would likely require a significant overhaul, including a similar change to IdOffsetRange.

[glwagner]

The PR would likely require a significant overhaul, including a similar change to IdOffsetRange.

I think we would be ok with preserving axes as Int range, only changing the type of offset and promoting appropriately.

But I think @jishnub is right that we will need a new type parameter in IdOffsetRange to remove the restriction that offset is the same Int type as the eltype of parent:

OffsetArrays.jl/src/axes.jl

Lines 78 to 95 in 5972e9e

	struct IdOffsetRange{T<:Integer,I<:AbstractUnitRange{T}} <: AbstractUnitRange{T}
	parent::I
	offset::T
	function IdOffsetRange{T,I}(r::I, offset::T) where {T<:Integer,I<:AbstractUnitRange{T}}
	_bool_check(T, r, offset)
	new{T,I}(r, offset)
	end
	#= This method is necessary to avoid a StackOverflowError in IdOffsetRange{T,I}(r::IdOffsetRange, offset::Integer).
	The type signature in that method is more specific than IdOffsetRange{T,I}(r::I, offset::T),
	so it ends up calling itself if I <: IdOffsetRange.
	=#
	function IdOffsetRange{T,IdOffsetRange{T,I}}(r::IdOffsetRange{T,I}, offset::T) where {T<:Integer,I<:AbstractUnitRange{T}}
	_bool_check(T, r, offset)
	new{T,IdOffsetRange{T,I}}(r, offset)
	end
	end

[simone-silvestri]

Do we necessarily need to change IdOffsetRange?
We are interested in having offsets of variable integer size, but I think the axes can stay Int64.

I have a tentative PR where almost all tests pass except for the Aqua test and this test

    @testset "issue #235" begin
        Vec64  = zeros(6)
        ind_a_64 = 3
        ind_a_32 =Int32.(ind_a_64)
        @test reshape(Vec64, ind_a_32, :) == reshape(Vec64, ind_a_64, :)
    end

which I am not sure why returns a StackOverflow. I will open the PR but I am not extremely sure about the downstream implication of the changes I am making.