Rigorous index handling: boundschecks and indexstyle

Project.toml

@@ -1,7 +1,7 @@
 name = "SparseArraysBase"
 uuid = "0d5efcca-f356-4864-8770-e1ed8d78f208"
 authors = ["ITensor developers <[email protected]> and contributors"]
-version = "0.5.4"
+version = "0.5.5"
 
 [deps]
 Accessors = "7d9f7c33-5ae7-4f3b-8dc6-eff91059b697"

README.md

@@ -89,7 +89,7 @@ using Dictionaries: IndexError
 @test isstored(a, 1, 2)
 @test setstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
 @test_throws IndexError setstoredindex!(copy(a), 21, 2, 1)
-@test setunstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
+@test_throws IndexError setunstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
 @test storedlength(a) == 1
 @test issetequal(storedpairs(a), [CartesianIndex(1, 2) => 12])
 @test issetequal(storedvalues(a), [12])

examples/README.jl

@@ -86,7 +86,7 @@ using Dictionaries: IndexError
 @test isstored(a, 1, 2)
 @test setstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
 @test_throws IndexError setstoredindex!(copy(a), 21, 2, 1)
-@test setunstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
+@test_throws IndexError setunstoredindex!(copy(a), 21, 1, 2) == [0 21; 0 0]
 @test storedlength(a) == 1
 @test issetequal(storedpairs(a), [CartesianIndex(1, 2) => 12])
 @test issetequal(storedvalues(a), [12])

src/SparseArraysBase.jl

@@ -19,6 +19,7 @@ export SparseArrayDOK,
 
 include("abstractsparsearrayinterface.jl")
 include("sparsearrayinterface.jl")
+include("indexing.jl")
 include("wrappers.jl")
 include("abstractsparsearray.jl")
 include("sparsearraydok.jl")

src/abstractsparsearrayinterface.jl

@@ -32,87 +32,6 @@
 
 # Minimal interface for `SparseArrayInterface`.
 # Fallbacks for dense/non-sparse arrays.
-function isstored(a::AbstractArray{<:Any,N}, I::Vararg{Int,N}) where {N}
-  @_propagate_inbounds_meta
-  @boundscheck checkbounds(a, I...)
-  return true
-end
-function isstored(a::AbstractArray, I::Int)
-  @_propagate_inbounds_meta
-  return isstored(a, Tuple(CartesianIndices(a)[I])...)
-end
-function isstored(a::AbstractArray, I::Int...)
-  @_propagate_inbounds_meta
-  @boundscheck checkbounds(a, I...)
-  I′ = ntuple(i -> I[i], ndims(a))
-  return isstored(a, I′...)
-end
-
-@interface ::AbstractArrayInterface eachstoredindex(a::AbstractArray) = eachindex(a)
-@interface ::AbstractArrayInterface getstoredindex(a::AbstractArray, I::Int...) =
-  getindex(a, I...)
-@interface ::AbstractArrayInterface function setstoredindex!(
-  a::AbstractArray, value, I::Int...
-)
-  setindex!(a, value, I...)
-  return a
-end
-# TODO: Should this error by default if the value at the index
-# is stored? It could be disabled with something analogous
-# to `checkbounds`, like `checkstored`/`checkunstored`.
-@interface ::AbstractArrayInterface function setunstoredindex!(
-  a::AbstractArray, value, I::Int...
-)
-  # TODO: Make this a `MethodError`?
-  return error("Not implemented.")
-end
-
-# TODO: Use `Base.to_indices`?
-isstored(a::AbstractArray, I::CartesianIndex) = isstored(a, Tuple(I)...)
-# TODO: Use `Base.to_indices`?
-getstoredindex(a::AbstractArray, I::CartesianIndex) = getstoredindex(a, Tuple(I)...)
-# TODO: Use `Base.to_indices`?
-getunstoredindex(a::AbstractArray, I::CartesianIndex) = getunstoredindex(a, Tuple(I)...)
-# TODO: Use `Base.to_indices`?
-function setstoredindex!(a::AbstractArray, value, I::CartesianIndex)
-  return setstoredindex!(a, value, Tuple(I)...)
-end
-# TODO: Use `Base.to_indices`?
-function setunstoredindex!(a::AbstractArray, value, I::CartesianIndex)
-  return setunstoredindex!(a, value, Tuple(I)...)
-end
-
-# Interface defaults.
-# TODO: Have a fallback that handles element types
-# that don't define `zero(::Type)`.
-@interface ::AbstractArrayInterface getunstoredindex(a::AbstractArray, I::Int...) =
-  zero(eltype(a))
-
-# DerivableInterfacesd interface.
-@interface ::AbstractArrayInterface storedlength(a::AbstractArray) = length(storedvalues(a))
-@interface ::AbstractArrayInterface storedpairs(a::AbstractArray) =
-  map(I -> I => getstoredindex(a, I), eachstoredindex(a))
-
-@interface ::AbstractArrayInterface function eachstoredindex(as::AbstractArray...)
-  return eachindex(as...)
-end
-
-@interface ::AbstractArrayInterface storedvalues(a::AbstractArray) = a
-
-# Automatically derive the interface for all `AbstractArray` subtypes.
-# TODO: Define `SparseArrayInterfaceOps` derivable trait and rewrite this
-# as `@derive AbstractArray SparseArrayInterfaceOps`.
-@derive (T=AbstractArray,) begin
-  SparseArraysBase.eachstoredindex(::T)
-  SparseArraysBase.eachstoredindex(::T...)
-  SparseArraysBase.getstoredindex(::T, ::Int...)
-  SparseArraysBase.getunstoredindex(::T, ::Int...)
-  SparseArraysBase.setstoredindex!(::T, ::Any, ::Int...)
-  SparseArraysBase.setunstoredindex!(::T, ::Any, ::Int...)
-  SparseArraysBase.storedlength(::T)
-  SparseArraysBase.storedpairs(::T)
-  SparseArraysBase.storedvalues(::T)
-end
 
 # TODO: Add `ndims` type parameter, like `Base.Broadcast.AbstractArrayStyle`.
 # TODO: This isn't used to define interface functions right now.
@@ -160,56 +79,9 @@
 end
 StoredValues(a::AbstractArray) = StoredValues(a, to_vec(eachstoredindex(a)))
 Base.size(a::StoredValues) = size(a.storedindices)
-Base.getindex(a::StoredValues, I::Int) = getstoredindex(a.array, a.storedindices[I])
-function Base.setindex!(a::StoredValues, value, I::Int)
-  return setstoredindex!(a.array, value, a.storedindices[I])
-end
-
-@interface ::AbstractSparseArrayInterface function isstored(
-  a::AbstractArray{<:Any,N}, I::Vararg{Int,N}
-) where {N}
-  return CartesianIndex(I) in eachstoredindex(a)
-end
-
-@interface ::AbstractSparseArrayInterface storedvalues(a::AbstractArray) = StoredValues(a)
-
-@interface ::AbstractSparseArrayInterface function eachstoredindex(
-  a1::AbstractArray, a2::AbstractArray, a_rest::AbstractArray...
-)
-  # TODO: Make this more customizable, say with a function
-  # `combine/promote_storedindices(a1, a2)`.
-  return union(eachstoredindex.((a1, a2, a_rest...))...)
-end
-
-@interface ::AbstractSparseArrayInterface function eachstoredindex(a::AbstractArray)
-  # TODO: Use `MethodError`?
-  return error("Not implemented.")
-end
-
-# We restrict to `I::Vararg{Int,N}` to allow more general functions to handle trailing
-# indices and linear indices.
-@interface ::AbstractSparseArrayInterface function Base.getindex(
-  a::AbstractArray{<:Any,N}, I::Vararg{Int,N}
-) where {N}
-  !isstored(a, I...) && return getunstoredindex(a, I...)
-  return getstoredindex(a, I...)
-end
-
-# We restrict to `I::Vararg{Int,N}` to allow more general functions to handle trailing
-# indices and linear indices.
-@interface ::AbstractSparseArrayInterface function Base.setindex!(
-  a::AbstractArray{<:Any,N}, value, I::Vararg{Int,N}
-) where {N}
-  if !isstored(a, I...)
-    # Don't set the value if it is zero, but only check
-    # if it is zero if the elements are numbers since otherwise
-    # it may be nontrivial to check.
-    eltype(a) <: Number && iszero(value) && return a
-    setunstoredindex!(a, value, I...)
-    return a
-  end
-  setstoredindex!(a, value, I...)
-  return a
+@inline Base.getindex(a::StoredValues, I::Int) = getindex(a.array, a.storedindices[I])
+@inline function Base.setindex!(a::StoredValues, value, I::Int)
+  return setindex!(a.array, value, a.storedindices[I])
 end
 
 # TODO: This may need to be defined in `sparsearraydok.jl`, after `SparseArrayDOK`