-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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.
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
Deprecate generic stride implementation (#17812), allow AbstractArrays to use BLAS/LAPACK routines (#25247) #25321
Changes from 6 commits
5c5d369
13eb90f
acaf1c2
93998ad
0858576
903c42e
4c2885d
d353ab0
31d4ce6
9c8fb44
d1086e0
579ceec
337785d
a558d76
7ce3036
346605b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3489,6 +3489,20 @@ end | |
@deprecate getq(F::Factorization) F.Q | ||
end | ||
|
||
# Issues #17812 Remove default stride implementation | ||
function stride(a::AbstractArray, i::Integer) | ||
depwarn("`stride(a::AbstractArray, i)` is deprecated for general arrays. Override `stride` for custom array types that implement the strided array interface.", :stride) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this is insufficiently cautionary. Maybe
|
||
if i > ndims(a) | ||
return length(a) | ||
end | ||
s = 1 | ||
for n = 1:(i-1) | ||
s *= size(a, n) | ||
end | ||
return s | ||
_stride(a, i) | ||
end | ||
|
||
# END 0.7 deprecations | ||
|
||
# BEGIN 1.0 deprecations | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,8 +36,8 @@ import Base: USE_BLAS64, abs, acos, acosh, acot, acoth, acsc, acsch, adjoint, as | |
cosh, cot, coth, csc, csch, eltype, exp, findmax, findmin, fill!, floor, getindex, hcat, | ||
getproperty, imag, inv, isapprox, isone, IndexStyle, kron, length, log, map, ndims, | ||
oneunit, parent, power_by_squaring, print_matrix, promote_rule, real, round, sec, sech, | ||
setindex!, show, similar, sin, sincos, sinh, size, sqrt, tan, tanh, transpose, trunc, | ||
typed_hcat, vec | ||
setindex!, show, similar, sin, sincos, sinh, size, size_to_strides, sqrt, StridedReinterpretArray, | ||
StridedReshapedArray, strides, stride, tan, tanh, transpose, trunc, typed_hcat, vec | ||
using Base: hvcat_fill, iszero, IndexLinear, _length, promote_op, promote_typeof, | ||
@propagate_inbounds, @pure, reduce, typed_vcat | ||
# We use `_length` because of non-1 indices; releases after julia 0.5 | ||
|
@@ -216,9 +216,40 @@ end | |
# Check that stride of matrix/vector is 1 | ||
# Writing like this to avoid splatting penalty when called with multiple arguments, | ||
# see PR 16416 | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the new IPO constant propagation do we need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This happens automatically at compile time? Cool! I'm not sure how to test for performance regression or that it's doing the right behaviour, so I might hold off on that change (since it's independent of this PR). |
||
stride1(A) -> Int | ||
|
||
Return the distance between successive array elements | ||
in dimension 1 in units of element size. | ||
|
||
# Examples | ||
```jldoctest | ||
julia> A = [1,2,3,4] | ||
4-element Array{Int64,1}: | ||
1 | ||
2 | ||
3 | ||
4 | ||
|
||
julia> Base.LinAlg.stride1(A) | ||
1 | ||
|
||
julia> B = view(A, 2:2:4) | ||
2-element view(::Array{Int64,1}, 2:2:4) with eltype Int64: | ||
2 | ||
4 | ||
|
||
julia> Base.LinAlg.stride1(B) | ||
2 | ||
``` | ||
""" | ||
stride1(x) = stride(x,1) | ||
stride1(x::Array) = 1 | ||
stride1(x::DenseArray) = stride(x, 1)::Int | ||
|
||
@inline chkstride1(A...) = _chkstride1(true, A...) | ||
@noinline _chkstride1(ok::Bool) = ok || error("matrix does not have contiguous columns") | ||
@inline _chkstride1(ok::Bool, A, B...) = _chkstride1(ok & (stride(A, 1) == 1), B...) | ||
@inline _chkstride1(ok::Bool, A, B...) = _chkstride1(ok & (stride1(A) == 1), B...) | ||
|
||
""" | ||
LinAlg.checksquare(A) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -255,11 +255,11 @@ IndexStyle(::Type{<:SubArray}) = IndexCartesian() | |
# so they are well-defined even for non-linear memory layouts | ||
strides(V::SubArray) = substrides(V.parent, V.indices) | ||
|
||
substrides(parent, I::Tuple) = substrides(1, parent, 1, I) | ||
substrides(parent, I::Tuple) = substrides(stride(parent, 1), parent, 1, I) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I imagine that many implementations of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Maybe something like: substrides(parent, I) = substrides(parent, strides(parent), I)
substrides(parent, strds, ::Tuple{}) = ()
substrides(parent, strds, I::Tuple{ScalarIndex, Vararg{Any}}) = (substrides(parent, tail(strds), tail(I))...,)
substrides(parent, strds, I::Tuple{Slice, Vararg{Any}}) = (first(strds), substrides(parent, tail(strds), tail(I))...)
substrides(parent, strds, I::Tuple{AbstractRange, Vararg{Any}}) = (first(strds)*step(I[1]), substrides(parent, tail(strds), tail(I))...)
substrides(parent, strds, I::Tuple{Any, Vararg{Any}}) = throw(ArgumentError("strides is invalid for SubArrays with indices of type $(typeof(I[1]))")) Does |
||
substrides(s, parent, dim, ::Tuple{}) = () | ||
substrides(s, parent, dim, I::Tuple{ScalarIndex, Vararg{Any}}) = (substrides(s*size(parent, dim), parent, dim+1, tail(I))...,) | ||
substrides(s, parent, dim, I::Tuple{Slice, Vararg{Any}}) = (s, substrides(s*size(parent, dim), parent, dim+1, tail(I))...) | ||
substrides(s, parent, dim, I::Tuple{AbstractRange, Vararg{Any}}) = (s*step(I[1]), substrides(s*size(parent, dim), parent, dim+1, tail(I))...) | ||
substrides(s, parent, dim, I::Tuple{ScalarIndex, Vararg{Any}}) = (substrides(stride(parent, dim+1), parent, dim+1, tail(I))...,) | ||
substrides(s, parent, dim, I::Tuple{Slice, Vararg{Any}}) = (s, substrides(stride(parent, dim+1), parent, dim+1, tail(I))...) | ||
substrides(s, parent, dim, I::Tuple{AbstractRange, Vararg{Any}}) = (s*step(I[1]), substrides(stride(parent, dim+1), parent, dim+1, tail(I))...) | ||
substrides(s, parent, dim, I::Tuple{Any, Vararg{Any}}) = throw(ArgumentError("strides is invalid for SubArrays with indices of type $(typeof(I[1]))")) | ||
|
||
stride(V::SubArray, d::Integer) = d <= ndims(V) ? strides(V)[d] : strides(V)[end] * size(V)[end] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -391,6 +391,24 @@ something other than 1), you should specialize `indices`. You should also specia | |
so that the `dims` argument (ordinarily a `Dims` size-tuple) can accept `AbstractUnitRange` objects, | ||
perhaps range-types `Ind` of your own design. For more information, see [Arrays with custom indices](@ref). | ||
|
||
## [Strided Arrays] | ||
|
||
| Methods to implement | | Brief description | | ||
|:----------------------------------------------- |:-------------------------------------- |:------------------------------------------------------------------------------------- | | ||
| `stride(A, i::Int)` | | Return the distance in memory (in number of elements) between adjacent elements in dimension k. | | ||
| `Base.unsafe_convert(::Type{Ptr{T}}, A)` | | Return the native address of an array. | | ||
|
||
|
||
A strided array is a sub-type of `AbstractArray` whose entries are stored in memory with fixed strides. | ||
Provided the element type of the array is compatible with BLAS, a strided array can utilize BLAS and LAPACK routines | ||
for more efficient linear algebra routines. | ||
|
||
A typical example of a user defined strided array is one that wraps a standard `Array` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hyphenate "user-defined" (hyphenate compound adjectives). Don't hyphenate "subtype". |
||
with additional structure. | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd add a warning here too about how dangerous it is to implement these methods if the underlying storage is not actually strided. Otherwise the following might happen:
It might be useful to give some concrete examples of arrays and categorize them as to whether they have strided memory representation. Examples: 1:5 # not strided (there is no storage associated with this array, could segfault if implemented `strides`)
collect(1:5) # is strided
A = [1 5; 2 6; 3 7; 4 8] # is strided
V = view(A, 1:2, :) # is strided
V = view(A, 1:2:4, :) # is strided
V = view(A, [1,2,4], :) # is not strided |
||
|
||
|
||
## [Broadcasting](@id man-interfaces-broadcasting) | ||
|
||
| Methods to implement | Brief description | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cumprod of the size? But it's not quite that, it's
cumprod([1;collect(size(A))[1:end-1]])