-
-
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
Rewrite mul! to dispatch based on memory layout, not matrix type #25558
Closed
Closed
Changes from all commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
e86e8bb
Added MemoryLayout, rewrote mul! to be based on memory layout
dlfivefifty 1d80d44
MemoryLayout implemented for instances not types, improved mul! type …
dlfivefifty 81c2f92
Added other memory layouts, updated triangular
dlfivefifty 84fc2ee
fix ambiguity
dlfivefifty c65a568
fix UndefVar error
dlfivefifty f12129d
Merge branch 'master' of https://github.com/JuliaLang/julia into dl/a…
f467664
Merge branch 'master' of https://github.com/JuliaLang/julia into dl/a…
1387afc
Restore mapslices (for now)
4ff991d
order of generalized eigvals appears to be brittle, so sort before co…
9bbdc8f
merge
dlfivefifty c4d93e5
merge triangular
dlfivefifty 9d23927
Fix mul2! usages
dlfivefifty cac81bc
Fix transpose/adjoint MemoryLayout, add tests, add DenseLayout
dlfivefifty c745821
dot, dotu dispatch, remove special * for Adjoint/Transpose
dlfivefifty fe55aad
Add MemoryLayout for symmetric, add docs
dlfivefifty 807644d
Merge branch 'master' of https://github.com/JuliaLang/julia into dl/a…
dlfivefifty 30d5ad8
Fix whitespace
dlfivefifty a67eebe
Fix symmetric ambiguities
dlfivefifty 725ab0e
Merge master
dlfivefifty 3f2528d
Override MemoryLayout for all DenseVector/Matrices (included SharedAr…
15238b6
Rename layouts to DenseColumnMajor, DenseColumnsStridedRows, DenseRow…
3e1e4c4
Add ConjLayout to replace ConjDenseColumns, and others
dlfivefifty 64e8609
add strides for DenseRowMajor
dlfivefifty ce99b1b
strides for BitArray, conj of triangular layouts
dlfivefifty 1a454fd
merge master
dlfivefifty 33f4e48
mul1! -> rmul!, mul2! -> lmul!
dlfivefifty 37c44d5
Redesign TriangularLayouts and Symmetric/HermitianLayout, add tests t…
dlfivefifty c5ddd01
Move MemoryLayout routines to Base
dlfivefifty 8c4d4cd
merge master
dlfivefifty 482939a
merge dense
dlfivefifty 01047c8
Merge branch 'master' of https://github.com/JuliaLang/julia into dl/a…
dlfivefifty 3618a39
DenseColumns -> AbstractColumnMajor
0b0eb44
Merge master
bad7814
MemoryLayout{T} -> MemoryLayout, as well as subtypes
6110ccb
Fix vecdot, be more conservative in dispatch for symmetriclayout, etc…
dlfivefifty dfcc856
Fix vecdot ambiguity
dlfivefifty 8ad8a35
submemorylayout -> subarraylayout, MemoryLayout(::DenseArray) restore…
5d55e48
first attempt at arbitrary d
74c7f67
subarraylayout now works with arb d
dlfivefifty d723b1a
more ambiguities fixed
dlfivefifty a0cd467
add RowMajorArray tests, update memory layout docs for arbitrary dime…
681f73b
Add _mul(A, B, memlayA, memlayB) to simplify * overloads
9d0f50b
view(UpperTriangular(A), :, Base.OneTo(n)) (and similar) now conform …
3413fba
remove white space
dlfivefifty ed88786
merge master
dlfivefifty 5535185
Add Increasing/DecreasingStrides
dlfivefifty 53c2879
merge master
7b19e38
Add `Base.MemoryLayout(A)` as optional override, fix docs for MemoryL…
09aa094
Merge branch 'master' of https://github.com/JuliaLang/julia into dl/a…
f2f1b8f
Add NEWS item
e19f1a5
fixes for mbauman
dlfivefifty 196b040
Merge branch 'dl/arraymemorylayout' of https://github.com/dlfivefifty…
dlfivefifty d386ef3
Merge branch 'master' of https://github.com/JuliaLang/julia into dl/a…
dlfivefifty f2bc361
Merge master
dlfivefifty File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -345,6 +345,138 @@ size_to_strides(s, d) = (s,) | |
size_to_strides(s) = () | ||
|
||
|
||
abstract type MemoryLayout end | ||
struct UnknownLayout <: MemoryLayout end | ||
abstract type AbstractStridedLayout <: MemoryLayout end | ||
abstract type AbstractIncreasingStrides <: AbstractStridedLayout end | ||
abstract type AbstractColumnMajor <: AbstractIncreasingStrides end | ||
struct DenseColumnMajor <: AbstractColumnMajor end | ||
struct ColumnMajor <: AbstractColumnMajor end | ||
struct IncreasingStrides <: AbstractIncreasingStrides end | ||
abstract type AbstractDecreasingStrides <: AbstractStridedLayout end | ||
abstract type AbstractRowMajor <: AbstractDecreasingStrides end | ||
struct DenseRowMajor <: AbstractRowMajor end | ||
struct RowMajor <: AbstractRowMajor end | ||
struct DecreasingStrides <: AbstractIncreasingStrides end | ||
struct StridedLayout <: AbstractStridedLayout end | ||
|
||
""" | ||
UnknownLayout() | ||
|
||
is returned by `MemoryLayout(A)` if it is unknown how the entries of an array `A` | ||
are stored in memory. | ||
""" | ||
UnknownLayout | ||
|
||
""" | ||
AbstractStridedLayout | ||
|
||
is an abstract type whose subtypes are returned by `MemoryLayout(A)` | ||
if an array `A` has storage laid out at regular offsets in memory, | ||
and which can therefore be passed to external C and Fortran functions expecting | ||
this memory layout. | ||
|
||
Julia's internal linear algebra machinery will automatically (and invisibly) | ||
dispatch to BLAS and LAPACK routines if the memory layout is BLAS compatible and | ||
the element type is a `Float32`, `Float64`, `ComplexF32`, or `ComplexF64`. | ||
In this case, one must implement the strided array interface, which requires | ||
overrides of `strides(A::MyMatrix)` and `unknown_convert(::Type{Ptr{T}}, A::MyMatrix)`. | ||
""" | ||
AbstractStridedLayout | ||
|
||
""" | ||
DenseColumnMajor() | ||
|
||
is returned by `MemoryLayout(A)` if an array `A` has storage in memory | ||
equivalent to an `Array`, so that `stride(A,1) == 1` and | ||
`stride(A,i) ≡ size(A,i-1) * stride(A,i-1)` for `2 ≤ i ≤ ndims(A)`. In particular, | ||
if `A` is a matrix then `strides(A) == `(1, size(A,1))`. | ||
|
||
Arrays with `DenseColumnMajor` memory layout must conform to the `DenseArray` interface. | ||
""" | ||
DenseColumnMajor | ||
|
||
""" | ||
ColumnMajor() | ||
|
||
is returned by `MemoryLayout(A)` if an array `A` has storage in memory | ||
as a column major array, so that `stride(A,1) == 1` and | ||
`stride(A,i) ≥ size(A,i-1) * stride(A,i-1)` for `2 ≤ i ≤ ndims(A)`. | ||
|
||
Arrays with `ColumnMajor` memory layout must conform to the `DenseArray` interface. | ||
""" | ||
ColumnMajor | ||
|
||
""" | ||
IncreasingStrides() | ||
|
||
is returned by `MemoryLayout(A)` if an array `A` has storage in memory | ||
as a strided array with increasing strides, so that `stride(A,1) ≥ 1` and | ||
`stride(A,i) ≥ size(A,i-1) * stride(A,i-1)` for `2 ≤ i ≤ ndims(A)`. | ||
""" | ||
IncreasingStrides | ||
|
||
""" | ||
DenseRowMajor() | ||
|
||
is returned by `MemoryLayout(A)` if an array `A` has storage in memory | ||
as a row major array with dense entries, so that `stride(A,ndims(A)) == 1` and | ||
`stride(A,i) ≡ size(A,i+1) * stride(A,i+1)` for `1 ≤ i ≤ ndims(A)-1`. In particular, | ||
if `A` is a matrix then `strides(A) == `(size(A,2), 1)`. | ||
""" | ||
DenseRowMajor | ||
|
||
""" | ||
RowMajor() | ||
|
||
is returned by `MemoryLayout(A)` if an array `A` has storage in memory | ||
as a row major array, so that `stride(A,ndims(A)) == 1` and | ||
stride(A,i) ≥ size(A,i+1) * stride(A,i+1)` for `1 ≤ i ≤ ndims(A)-1`. | ||
|
||
If `A` is a matrix with `RowMajor` memory layout, then | ||
`transpose(A)` should return a matrix whose layout is `ColumnMajor`. | ||
""" | ||
RowMajor | ||
|
||
""" | ||
DecreasingStrides() | ||
|
||
is returned by `MemoryLayout(A)` if an array `A` has storage in memory | ||
as a strided array with decreasing strides, so that `stride(A,ndims(A)) ≥ 1` and | ||
stride(A,i) ≥ size(A,i+1) * stride(A,i+1)` for `1 ≤ i ≤ ndims(A)-1`. | ||
""" | ||
DecreasingStrides | ||
|
||
""" | ||
StridedLayout() | ||
|
||
is returned by `MemoryLayout(A)` if an array `A` has storage laid out at regular | ||
offsets in memory. `Array`s with `StridedLayout` must conform to the `DenseArray` interface. | ||
""" | ||
StridedLayout | ||
|
||
""" | ||
MemoryLayout(A) | ||
|
||
specifies the layout in memory for an array `A`. When | ||
you define a new `AbstractArray` type, you can choose to override | ||
`MemoryLayout` to indicate how an array is stored in memory. | ||
For example, if your matrix is column major with `stride(A,2) == size(A,1)`, | ||
then override as follows: | ||
|
||
Base.MemoryLayout(::MyMatrix) = Base.DenseColumnMajor() | ||
|
||
The default is `Base.UnknownLayout()` to indicate that the layout | ||
in memory is unknown. | ||
|
||
Julia's internal linear algebra machinery will automatically (and invisibly) | ||
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 say this part in the |
||
dispatch to BLAS and LAPACK routines if the memory layout is compatible. | ||
""" | ||
MemoryLayout(A::AbstractArray{T}) where T = UnknownLayout() | ||
MemoryLayout(A::DenseArray{T}) where T = DenseColumnMajor() | ||
|
||
|
||
|
||
function isassigned(a::AbstractArray, i::Int...) | ||
try | ||
a[i...] | ||
|
@@ -379,6 +511,50 @@ function trailingsize(inds::Indices) | |
prod(map(unsafe_length, inds)) | ||
end | ||
|
||
## Traits for array types ## | ||
|
||
abstract type IndexStyle end | ||
struct IndexLinear <: IndexStyle end | ||
struct IndexCartesian <: IndexStyle end | ||
|
||
""" | ||
IndexStyle(A) | ||
IndexStyle(typeof(A)) | ||
|
||
`IndexStyle` specifies the "native indexing style" for array `A`. When | ||
you define a new `AbstractArray` type, you can choose to implement | ||
either linear indexing or cartesian indexing. If you decide to | ||
implement linear indexing, then you must set this trait for your array | ||
type: | ||
|
||
Base.IndexStyle(::Type{<:MyArray}) = IndexLinear() | ||
|
||
The default is `IndexCartesian()`. | ||
|
||
Julia's internal indexing machinery will automatically (and invisibly) | ||
convert all indexing operations into the preferred style. This allows users | ||
to access elements of your array using any indexing style, even when explicit | ||
methods have not been provided. | ||
|
||
If you define both styles of indexing for your `AbstractArray`, this | ||
trait can be used to select the most performant indexing style. Some | ||
methods check this trait on their inputs, and dispatch to different | ||
algorithms depending on the most efficient access pattern. In | ||
particular, [`eachindex`](@ref) creates an iterator whose type depends | ||
on the setting of this trait. | ||
""" | ||
IndexStyle(A::AbstractArray) = IndexStyle(typeof(A)) | ||
IndexStyle(::Type{Union{}}) = IndexLinear() | ||
IndexStyle(::Type{<:AbstractArray}) = IndexCartesian() | ||
IndexStyle(::Type{<:Array}) = IndexLinear() | ||
IndexStyle(::Type{<:AbstractRange}) = IndexLinear() | ||
|
||
IndexStyle(A::AbstractArray, B::AbstractArray) = IndexStyle(IndexStyle(A), IndexStyle(B)) | ||
IndexStyle(A::AbstractArray, B::AbstractArray...) = IndexStyle(IndexStyle(A), IndexStyle(B...)) | ||
IndexStyle(::IndexLinear, ::IndexLinear) = IndexLinear() | ||
IndexStyle(::IndexStyle, ::IndexStyle) = IndexCartesian() | ||
|
||
|
||
## Bounds checking ## | ||
|
||
# The overall hierarchy is | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This still seems far too fine-grained. Yes, these are really cool and it's awesome that you can track them through some of our wrapper arrays, but you barely use them for the purposes of dispatching
mul
to BLAS. Really, BLAS only cares about a handful of things beyond the basic strided-ness:stride(A::AbstractMatrix, 1) == 1
)stride(A::AbstractMatrix, 2) == 1
)For the wrapper arrays (reshape and view) to be able to preserve stridedness through their transformations we additionally need to know if the arrays are wholly contiguous (
stride(vec(A), 1) == 1
orstride(vec(permutedims(A, reverse(ntuple(identity, ndims(A)))), 1) == 1
).You're going to hate me, but I really think this should just be:
Note that
gemm_wrapper!
already does value-based checking onstride(A, 1) == 1
, etc. I really don't see value in having these things encoded into a traitsystem. To the contrary: I think the additional complexity is just asking for trouble.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.
Let's handle renaming separately, as that's a trivial find/replace change.
So back to what it was originally? Weren't you the one to suggest adding
Increasing/DecreasingStrides
?Removing
ColumnMajor
loses functionality:V = view(A,1:5,1,2,3)
is known to beDenseColumnMajor
ifA
isColumnMajor
, so we can do, for example,reshape(V, (2,3))
and still use BLAS.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.
Yes, you're right — I should be using your existing terms here. I was sketching out lots of possibilities and this slowly diverged as I progressed. The core idea is to eliminate all
AbstractStridedLayout
subtypes except the general one andDenseRowMajor
andDenseColumnMajor
.Yup, I was the one to suggest the Increasing/DecreasingStrides terminology but my goal was to generalize/rename and simplify. I definitely don't want any nodes here that you don't actually use.
Yes, your exact example there is correct, but do you have any examples of
ColumnMajor
StridedArray
s that aren't views of aDense
structure? Because ifA
is aColumnMajor
strided view into aDenseColumnMajor
array, thenSubArray
"pops" the intermediate view and recomputes its indices in terms of the original parent, making that example work. Further, were I developing aColumnMajor
array type specifically, I think I'd be just fine adding specializations onview(A::MyColumnMajor, ::AbstractUnitRange, ...)
to impart the additional knowledge that's available in that case. If I'm wrong, we can add this back in in the future. We can document that this type tree is experimental and that the genericStridedLayout
shouldn't be used for dispatch as a more specific type might get returned in the future.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.
I regard relying on overloading of
view
as a no go: the initial motivation for this PR (which came up in other contexts subsequently) was that ifA
is banded thenview(A, ::UnitRange, ::UnitRange)
is also banded.Now one could override
view
to return aSubBandedMatrix
, which is what I did initially, but it results in tons of copy-and-paste of subarray.jl. The "correct" solution was to instead link into the currentSubArray
infrastructure.I think we should set aside the discussion of simplifying the type hierarchy until others weigh in, as there are a number of different needs here.