Skip to content

Commit 2550ece

Browse files
authored
Rigorous index handling: boundschecks and indexstyle (#52)
This PR implements indexing in a style more similar to how Base handles this, with canonical IndexStyle options and a more fixed callchain of converting different index types into eachother. Additionally, this (re)introduces boundschecking on sparse arrays.
1 parent ebb3f81 commit 2550ece

11 files changed

+472
-157
lines changed

Project.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name = "SparseArraysBase"
22
uuid = "0d5efcca-f356-4864-8770-e1ed8d78f208"
33
authors = ["ITensor developers <[email protected]> and contributors"]
4-
version = "0.5.4"
4+
version = "0.5.5"
55

66
[deps]
77
Accessors = "7d9f7c33-5ae7-4f3b-8dc6-eff91059b697"

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ using Dictionaries: IndexError
8989
@test isstored(a, 1, 2)
9090
@test setstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
9191
@test_throws IndexError setstoredindex!(copy(a), 21, 2, 1)
92-
@test setunstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
92+
@test_throws IndexError setunstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
9393
@test storedlength(a) == 1
9494
@test issetequal(storedpairs(a), [CartesianIndex(1, 2) => 12])
9595
@test issetequal(storedvalues(a), [12])

examples/README.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ using Dictionaries: IndexError
8686
@test isstored(a, 1, 2)
8787
@test setstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
8888
@test_throws IndexError setstoredindex!(copy(a), 21, 2, 1)
89-
@test setunstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
89+
@test_throws IndexError setunstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
9090
@test storedlength(a) == 1
9191
@test issetequal(storedpairs(a), [CartesianIndex(1, 2) => 12])
9292
@test issetequal(storedvalues(a), [12])

src/SparseArraysBase.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export SparseArrayDOK,
1919

2020
include("abstractsparsearrayinterface.jl")
2121
include("sparsearrayinterface.jl")
22+
include("indexing.jl")
2223
include("wrappers.jl")
2324
include("abstractsparsearray.jl")
2425
include("sparsearraydok.jl")

src/abstractsparsearrayinterface.jl

Lines changed: 3 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -32,87 +32,6 @@ end
3232

3333
# Minimal interface for `SparseArrayInterface`.
3434
# Fallbacks for dense/non-sparse arrays.
35-
function isstored(a::AbstractArray{<:Any,N}, I::Vararg{Int,N}) where {N}
36-
@_propagate_inbounds_meta
37-
@boundscheck checkbounds(a, I...)
38-
return true
39-
end
40-
function isstored(a::AbstractArray, I::Int)
41-
@_propagate_inbounds_meta
42-
return isstored(a, Tuple(CartesianIndices(a)[I])...)
43-
end
44-
function isstored(a::AbstractArray, I::Int...)
45-
@_propagate_inbounds_meta
46-
@boundscheck checkbounds(a, I...)
47-
I′ = ntuple(i -> I[i], ndims(a))
48-
return isstored(a, I′...)
49-
end
50-
51-
@interface ::AbstractArrayInterface eachstoredindex(a::AbstractArray) = eachindex(a)
52-
@interface ::AbstractArrayInterface getstoredindex(a::AbstractArray, I::Int...) =
53-
getindex(a, I...)
54-
@interface ::AbstractArrayInterface function setstoredindex!(
55-
a::AbstractArray, value, I::Int...
56-
)
57-
setindex!(a, value, I...)
58-
return a
59-
end
60-
# TODO: Should this error by default if the value at the index
61-
# is stored? It could be disabled with something analogous
62-
# to `checkbounds`, like `checkstored`/`checkunstored`.
63-
@interface ::AbstractArrayInterface function setunstoredindex!(
64-
a::AbstractArray, value, I::Int...
65-
)
66-
# TODO: Make this a `MethodError`?
67-
return error("Not implemented.")
68-
end
69-
70-
# TODO: Use `Base.to_indices`?
71-
isstored(a::AbstractArray, I::CartesianIndex) = isstored(a, Tuple(I)...)
72-
# TODO: Use `Base.to_indices`?
73-
getstoredindex(a::AbstractArray, I::CartesianIndex) = getstoredindex(a, Tuple(I)...)
74-
# TODO: Use `Base.to_indices`?
75-
getunstoredindex(a::AbstractArray, I::CartesianIndex) = getunstoredindex(a, Tuple(I)...)
76-
# TODO: Use `Base.to_indices`?
77-
function setstoredindex!(a::AbstractArray, value, I::CartesianIndex)
78-
return setstoredindex!(a, value, Tuple(I)...)
79-
end
80-
# TODO: Use `Base.to_indices`?
81-
function setunstoredindex!(a::AbstractArray, value, I::CartesianIndex)
82-
return setunstoredindex!(a, value, Tuple(I)...)
83-
end
84-
85-
# Interface defaults.
86-
# TODO: Have a fallback that handles element types
87-
# that don't define `zero(::Type)`.
88-
@interface ::AbstractArrayInterface getunstoredindex(a::AbstractArray, I::Int...) =
89-
zero(eltype(a))
90-
91-
# DerivableInterfacesd interface.
92-
@interface ::AbstractArrayInterface storedlength(a::AbstractArray) = length(storedvalues(a))
93-
@interface ::AbstractArrayInterface storedpairs(a::AbstractArray) =
94-
map(I -> I => getstoredindex(a, I), eachstoredindex(a))
95-
96-
@interface ::AbstractArrayInterface function eachstoredindex(as::AbstractArray...)
97-
return eachindex(as...)
98-
end
99-
100-
@interface ::AbstractArrayInterface storedvalues(a::AbstractArray) = a
101-
102-
# Automatically derive the interface for all `AbstractArray` subtypes.
103-
# TODO: Define `SparseArrayInterfaceOps` derivable trait and rewrite this
104-
# as `@derive AbstractArray SparseArrayInterfaceOps`.
105-
@derive (T=AbstractArray,) begin
106-
SparseArraysBase.eachstoredindex(::T)
107-
SparseArraysBase.eachstoredindex(::T...)
108-
SparseArraysBase.getstoredindex(::T, ::Int...)
109-
SparseArraysBase.getunstoredindex(::T, ::Int...)
110-
SparseArraysBase.setstoredindex!(::T, ::Any, ::Int...)
111-
SparseArraysBase.setunstoredindex!(::T, ::Any, ::Int...)
112-
SparseArraysBase.storedlength(::T)
113-
SparseArraysBase.storedpairs(::T)
114-
SparseArraysBase.storedvalues(::T)
115-
end
11635

11736
# TODO: Add `ndims` type parameter, like `Base.Broadcast.AbstractArrayStyle`.
11837
# TODO: This isn't used to define interface functions right now.
@@ -160,56 +79,9 @@ struct StoredValues{T,A<:AbstractArray{T},I} <: AbstractVector{T}
16079
end
16180
StoredValues(a::AbstractArray) = StoredValues(a, to_vec(eachstoredindex(a)))
16281
Base.size(a::StoredValues) = size(a.storedindices)
163-
Base.getindex(a::StoredValues, I::Int) = getstoredindex(a.array, a.storedindices[I])
164-
function Base.setindex!(a::StoredValues, value, I::Int)
165-
return setstoredindex!(a.array, value, a.storedindices[I])
166-
end
167-
168-
@interface ::AbstractSparseArrayInterface function isstored(
169-
a::AbstractArray{<:Any,N}, I::Vararg{Int,N}
170-
) where {N}
171-
return CartesianIndex(I) in eachstoredindex(a)
172-
end
173-
174-
@interface ::AbstractSparseArrayInterface storedvalues(a::AbstractArray) = StoredValues(a)
175-
176-
@interface ::AbstractSparseArrayInterface function eachstoredindex(
177-
a1::AbstractArray, a2::AbstractArray, a_rest::AbstractArray...
178-
)
179-
# TODO: Make this more customizable, say with a function
180-
# `combine/promote_storedindices(a1, a2)`.
181-
return union(eachstoredindex.((a1, a2, a_rest...))...)
182-
end
183-
184-
@interface ::AbstractSparseArrayInterface function eachstoredindex(a::AbstractArray)
185-
# TODO: Use `MethodError`?
186-
return error("Not implemented.")
187-
end
188-
189-
# We restrict to `I::Vararg{Int,N}` to allow more general functions to handle trailing
190-
# indices and linear indices.
191-
@interface ::AbstractSparseArrayInterface function Base.getindex(
192-
a::AbstractArray{<:Any,N}, I::Vararg{Int,N}
193-
) where {N}
194-
!isstored(a, I...) && return getunstoredindex(a, I...)
195-
return getstoredindex(a, I...)
196-
end
197-
198-
# We restrict to `I::Vararg{Int,N}` to allow more general functions to handle trailing
199-
# indices and linear indices.
200-
@interface ::AbstractSparseArrayInterface function Base.setindex!(
201-
a::AbstractArray{<:Any,N}, value, I::Vararg{Int,N}
202-
) where {N}
203-
if !isstored(a, I...)
204-
# Don't set the value if it is zero, but only check
205-
# if it is zero if the elements are numbers since otherwise
206-
# it may be nontrivial to check.
207-
eltype(a) <: Number && iszero(value) && return a
208-
setunstoredindex!(a, value, I...)
209-
return a
210-
end
211-
setstoredindex!(a, value, I...)
212-
return a
82+
@inline Base.getindex(a::StoredValues, I::Int) = getindex(a.array, a.storedindices[I])
83+
@inline function Base.setindex!(a::StoredValues, value, I::Int)
84+
return setindex!(a.array, value, a.storedindices[I])
21385
end
21486

21587
# TODO: This may need to be defined in `sparsearraydok.jl`, after `SparseArrayDOK`

0 commit comments

Comments
 (0)