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.

Sign up for GitHub

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

Generalized indices and arbitrary indexing support. #1

Merged
merged 5 commits into from
Dec 29, 2019
Merged

Generalized indices and arbitrary indexing support. #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f45ba49
Generalized indices and arbitrary indexing support.
yha Dec 24, 2019
1be6237
Moved array creation inside test set
yha Dec 25, 2019
6c94825
more matrix tests. renamed (i,j)=>(x,y)
yha Dec 25, 2019
bb1103a
Removed OffsetArrays dependency
yha Dec 28, 2019
caac046
Comment tweak
yha Dec 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
name = "CircularArrays"
uuid = "dcaa3502-af75-11e8-34c7-6b8fb8855653"
license = "MIT"
desc = "Arrays with fixed size and circular indexing."
url = "https://github.com/Vexatos/CircularArrays.jl"
authors = ["Vexatos <[email protected]>"]
license = "MIT"
url = "https://github.com/Vexatos/CircularArrays.jl"
version = "0.1.0"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
OffsetArrays = "6fe1bfb0-de20-5000-8ca7-80f57d26f881"

[targets]
test = ["Test"]
test = ["Test", "OffsetArrays"]
35 changes: 27 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
# CircularArrays.jl - Multi-dimensional arrays with fixed size and circular indexing

CircularArrays.jl is a small package adding the `CircularArray{T, N}` type which can be backed by any `AbstractArray{T, N}`. A `CircularArray` has a fixed size and features circular indexing across all dimensions: Indexing and assigning beyond its bounds is possible, as the end of the array is considered adjacent to its start; indices less than 1 are possible too. Iterators will still stop at the end of the array, and indexing using ranges is only possible with ranges within the bounds of the backing array.
CircularArrays.jl is a small package adding the `CircularArray{T, N}` type which can be backed by any `AbstractArray{T, N}`. A `CircularArray` has a fixed size and features circular indexing across all dimensions: Indexing and assigning beyond its bounds in both directions is possible, as the end of the array is considered adjacent to its start. `CircularArray`s have the same `axes` as the underlying backing array, and iterators only iterate over these indices.

The `CircularVector{T}` type is added as an alias for `CircularArray{T, 1}`.

```julia
# CircularArrays use mod1 for their circular behaviour.
array[index] == array[mod1(index, size)]
```

The following functions are provided.
The following constructors are provided.

```julia
# Initialize a CircularArray backed by any AbstractArray.
Expand All @@ -21,6 +16,30 @@ CircularVector(arr::AbstractArray{T, 1}) where T
CircularVector(initialValue::T, size::Int) where T
```

### Examples

```julia
julia> using CircularArrays
julia> a = CircularArray([1,2,3]);
julia> a[0:4]
5-element CircularArray{Int64,1}:
3
1
2
3
1
julia> using OffsetArrays
julia> i = OffsetArray(1:5,-2:2);
julia> a[i]
5-element CircularArray{Int64,1} with indices -2:2:
1
2
3
1
2
```


### License

CircularArrays.jl is licensed under the [MIT license](LICENSE.md). By using or interacting with this software in any way, you agree to the license of this software.
CircularArrays.jl is licensed under the [MIT license](LICENSE.md). By using or interacting with this software in any way, you agree to the license of this software.
24 changes: 20 additions & 4 deletions src/CircularArrays.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,35 @@ Alias for [`CircularArray{T,1}`](@ref).
"""
const CircularVector{T} = CircularArray{T, 1}

@inline clamp_bounds(arr::CircularArray, I::Tuple{Vararg{Int}})::AbstractArray{Int, 1} = map(dim -> mod1(I[dim], size(arr.data, dim)), eachindex(I))
# Copied from a method of Base.mod, for compatibility with Julia version < 1.3,
# where this method is not defined
_mod(i::Integer, r::AbstractUnitRange{<:Integer}) = mod(i-first(r), length(r)) + first(r)
Vexatos marked this conversation as resolved.
Show resolved Hide resolved
@inline clamp_bounds(arr::CircularArray, I::Tuple{Vararg{Int}})::AbstractArray{Int, 1} = map(Base.splat(_mod), zip(I, axes(arr.data)))

CircularArray(def::T, size) where T = CircularArray(fill(def, size))

@inline Base.getindex(arr::CircularArray, i::Int) = @inbounds getindex(arr.data, mod1(i, size(arr.data, 1)))
@inline Base.setindex!(arr::CircularArray, v, i::Int) = @inbounds setindex!(arr.data, v, mod1(i, size(arr.data, 1)))
@inline Base.getindex(arr::CircularArray, i::Int) = @inbounds getindex(arr.data, mod(i, Base.axes1(arr.data)))
@inline Base.setindex!(arr::CircularArray, v, i::Int) = @inbounds setindex!(arr.data, v, mod(i, Base.axes1(arr.data)))
@inline Base.getindex(arr::CircularArray, I::Vararg{Int}) = @inbounds getindex(arr.data, clamp_bounds(arr, I)...)
@inline Base.setindex!(arr::CircularArray, v, I::Vararg{Int}) = @inbounds setindex!(arr.data, v, clamp_bounds(arr, I)...)
@inline Base.size(arr::CircularArray) = size(arr.data)
@inline Base.axes(arr::CircularArray) = axes(arr.data)

@inline Base.checkbounds(::CircularArray, _...) = nothing

@inline _similar(arr::CircularArray, ::Type{T}, dims) where T = CircularArray(similar(arr.data,T,dims))
@inline Base.similar(arr::CircularArray, ::Type{T}, dims::Tuple{Base.DimOrInd, Vararg{Base.DimOrInd}}) where T = _similar(arr,T,dims)
Copy link
Owner

@Vexatos Vexatos Dec 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some more testing, I noticed that this line does not seem to be covered by the tests. Do you happen to have an example for where it might be used? Or did the addition of the ambiguity resolution for OffsetArrays cover this? Technically, this method is more broad than the one below since DimOrInd is more generic, but I cannot think of anything that would ever use this, short of someone implementing their own axis type, which would once again trigger the same ambiguity as below anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I added this before the ambiguity resolution.
I think you're right, and we will face an ambiguity error on similar when indexing with arrays defining a custom axis type in the "standard" way defined in the Julia docs. I tested this with CatIndices.BidirectionalVector, and indeed indexing c[bidi_vec] where c is a CircularArray triggers the ambiguity. If this more general similar method is removed the ambiguity disappears, but the result of indexing is no longer circular, which is inconsistent with c[a::Array].
I think maintaining the circularity in, e.g. c[1:5] is really useful, and it's unfortunate if we can't have it for c[i] where i has custom axes, but I'm not sure how it would be possible with how array indexing is designed in Base.

Copy link
Owner

@Vexatos Vexatos Jan 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I will keep this as it is for now. Thank you again for the contribution.

# Ambiguity resolution with Base
@inline Base.similar(arr::CircularArray, ::Type{T}, dims::Tuple{Int64,Vararg{Int64}}) where T = _similar(arr,T,dims)
# Ambiguity resolution with a type-pirating OffsetArrays method. See OffsetArrays issue #87.
# Ambiguity is triggered in the case similar(arr) where arr.data::OffsetArray.
# The OffsetAxis definition is copied from OffsetArrays.
const OffsetAxis = Union{Integer, UnitRange, Base.OneTo, Base.IdentityUnitRange, Colon}
@inline Base.similar(arr::CircularArray, ::Type{T}, dims::Tuple{OffsetAxis, Vararg{OffsetAxis}}) where T = _similar(arr,T,dims)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the hard dependency on OffsetArrays for this really necessary? I doubt that it would have to be. Why would this ambiguity resolution be needed at all? I'd like to avoid this hard dependency if possible, it's fine using the package for testing though.

Copy link
Contributor Author

@yha yha Dec 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason ambiguity resolution is needed is the similar method mentioned here: JuliaArrays/OffsetArrays.jl#87.
Without changes to Base or OffsetArrays, the only solutions I can see are:

  • a dependency on OffsetArrays
  • copy-pasting the OffsetAxis definition from OffsetArrays
  • defining our own axis type as suggested here Type-piracy in similar with OffsetAxis JuliaArrays/OffsetArrays.jl#87 (comment), and defining similar just for those. This would mean axes converts the underlying arrays axes to a custom range type, and similar does the opposite conversion to send the axes to the underlying array. This would solve the case similar(a::CircularArray) where a.data is offset, but not a direct call from user code like similar(a::CircularArray, -2:2, -2:2) (which would dispatch on the OffsetArrays definition and thus fail to produce a circular array). Perhaps I'm misunderstanding how this mechanism should be used. Maybe @timholy can elucidate? EDIT: actually, I tried this now and a lot of tests fail because indexing operations invoke similar with the index's axes, which sinks this whole approach.

Otherwise, I think changing Base and OffsetArrays as suggested here JuliaArrays/OffsetArrays.jl#87 (comment) would solve this problem too.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like an issue that needs to be fixed in OffsetArrays. I am not a fan of adding workarounds for problems in other packages in here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's absolutely necessary, I'd rather copy the OffsetAxis definition over. It's just a tuple, after all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JuliaArrays/OffsetArrays.jl#90 shows an example of defining your own axis type. You can specialize axes for the axes, as done in that PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be missing something, but as far as I can see, this OffsetArrays example works because it defines similar not only for its custom index types, but also for "standard" types appearing in the OffsetAxis union, because getindex(A,I...) generally calls similar with I's axes. We similarly can't avoid defining similar on the "standard" types, and then we're back with the ambiguity errors. If I only define similar for a custom axis type, then indexing ::CircularArray[::UnitRange] fails to produce a CircularArray (because it calls a similar method from base).
At this point, I don't see how a custom index type is helpful at all. For now I will resolve the ambiguity as in BlockArrays.jl.


CircularVector(data::AbstractArray{T, 1}) where T = CircularVector{T}(data)
CircularVector(def::T, size::Int) where T = CircularVector{T}(fill(def, size))

Base.IndexStyle(::Type{<:CircularVector}) = IndexLinear()

end
end
112 changes: 79 additions & 33 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,85 @@
using CircularArrays
using OffsetArrays
using Test

v1 = CircularVector(rand(Int64, 5))

@test IndexStyle(CircularArray) == IndexCartesian()
@test IndexStyle(CircularVector) == IndexLinear()

@test size(v1, 1) == 5
@test typeof(v1) == CircularVector{Int64}
@test isa(v1, CircularVector)
@test isa(v1, AbstractVector{Int})
@test !isa(v1, AbstractVector{String})
@test v1[2] == v1[2 + length(v1)]
v1[2] = 0
v1[3] = 0
@test v1[2] == v1[3]
@test_throws MethodError v1[2] = "Hello"

v2 = CircularVector("abcde", 5)

@test prod(v2) == "abcde"^5

@test_throws MethodError push!(v1, 15)

b_arr = [2 4 6 8; 10 12 14 16; 18 20 22 24]
a1 = CircularArray(b_arr)
@test size(a1) == (3, 4)
@test a1[2, 3] == 14
a1[2, 3] = 17
@test a1[2, 3] == 17
@test !isa(a1, CircularVector)
@test !isa(a1, AbstractVector)
@test isa(a1, AbstractArray)

@test size(reshape(a1, (2, 2, 3))) == (2, 2, 3)

a2 = CircularArray(4, (2, 3))
@test isa(a2, CircularArray{Int, 2})
@testset "vector" begin
data = rand(Int64, 5)
v1 = CircularVector(data)

@test size(v1, 1) == 5
@test typeof(v1) == CircularVector{Int64}
@test isa(v1, CircularVector)
@test isa(v1, AbstractVector{Int})
@test !isa(v1, AbstractVector{String})
@test v1[2] == v1[2 + length(v1)]

@test v1[0] == data[end]
@test v1[-4:10] == [data; data; data]
@test v1[-3:1][-1] == data[end]
@test v1[[true,false,true,false,true]] == v1[[1,3,0]]

v1copy = copy(v1)
v1_2 = v1[2]
v1[2] = 0
v1[3] = 0
@test v1[2] == v1[3] == 0
@test v1copy[2] == v1_2
@test v1copy[7] == v1_2
@test_throws MethodError v1[2] = "Hello"

v2 = CircularVector("abcde", 5)

@test prod(v2) == "abcde"^5

@test_throws MethodError push!(v1, 15)
end

@testset "matrix" begin
b_arr = [2 4 6 8; 10 12 14 16; 18 20 22 24]
a1 = CircularArray(b_arr)
@test size(a1) == (3, 4)
@test a1[2, 3] == 14
a1[2, 3] = 17
@test a1[2, 3] == 17
@test a1[-1, 7] == 17
@test a1[-1:5, 4:10][1, 4] == 17
@test a1[:, -1:-1][2, 1] == 17
@test !isa(a1, CircularVector)
@test !isa(a1, AbstractVector)
@test isa(a1, AbstractArray)

@test size(reshape(a1, (2, 2, 3))) == (2, 2, 3)

a2 = CircularArray(4, (2, 3))
@test isa(a2, CircularArray{Int, 2})
end

@testset "offset indices" begin
i = OffsetArray(1:5,-3)
a = CircularArray(i)
@test axes(a) == axes(i)
@test a[1] == 4
@test a[10] == a[-10] == a[0] == 3
@test a[-2:7] == [1:5; 1:5]
@test a[0:9] == [3:5; 1:5; 1:2]
@test a[1:10][-10] == 3
@test a[i] == OffsetArray([4,5,1,2,3],-3)

circ_a = circshift(a,3)
@test axes(circ_a) == axes(a)
@test circ_a[1:5] == 1:5

j = OffsetArray([true,false,true],1)
@test a[j] == [5,2]

data = reshape(1:9,3,3)
a = CircularArray(OffsetArray(data,-1,-1))
@test collect(a) == data
@test all(a[x,y] == data[mod1(x+1,3),mod1(y+1,3)] for x=-10:10, y=-10:10)
@test a[i,1] == CircularArray(OffsetArray([5,6,4,5,6],-2:2))
@test a[CartesianIndex.(i,i)] == CircularArray(OffsetArray([5,9,1,5,9],-2:2))
@test a[a .> 4] == 5:9
end