eliminate StoredArray

[stevengj]

Fixes #6212, #987; also, UniformScaling is no longer an AbstractArray (#5810).

Update: Changed to keep the name DenseArray, as there was no consensus for RegularArray

Also, BitArray and SharedArray are now subtypes of AbstractArray; they are not RegularArrays since they don't implement stride and don't have the requisite memory layout. (One of the main purposes of having RegularArray as a type is to know the memory layout of Ptr{T} conversions, as otherwise it is impossible to safely do a ccall.)

[stevengj]

Hmm, slight problem: our generic_matmul functions are only defined for StridedMatrix, for some reason. This means that matrix multiplications on BitArray no longer exists once BitArray is not a StridedMatrix.

One option would be to make BitArray a RegularArray (again) for now, and fix the BitArray issue later. This shouldn't be a regression, anyway.

[lindahua]

A easy way is to define generic_matmul for Union(StridedMatrix,BitMatrix).

[stevengj]

Why doesn't generic_matmul work for an arbitrary AbstractMatrix?

[lindahua]

Yes, I think it should work with AbstractMatrix as it seems to only uses size and getindex.

[carlobaldassi]

Why doesn't generic_matmul work for an arbitrary AbstractMatrix?

I asked the same thing once, see this discussion. In a nutshell, the original argument was that rather then having general fallback algorithms (in particular for linear algebra) which would perform horribly on some array types (e.g. DArrays, SparseMatrixCSC), it was preferable to restrict the signature to those types where it was known that performance could be reasonable. This was one of the reasons for wanting some super-type incuding Arrays and BitArrays but not other array types whose element access time was not uniform, which led to the introduction of DenseArray.

More "historical" references: #750, #987.

[lindahua]

Under the new definition, getindex takes Õ(1), and thus generic_matmul would take Õ(mnk) for matrices m x k and k x n. It is definitely not great for some matrices, but still have a reasonable bound.

It don't think it would be a problem to have fallback methods using permissive interface, as long as it still yields correct results. When efficient multiplication between certain types of matrices becomes needed, one can always add a specialized method.

The same apply to many other functions: +(AbstractArray, AbstractArray), sum(AbstractArray), etc. The performance would be less than desirable if you apply these generic fallback to sparse matrices or distributed matrices. However, it does not seem to have any issues having these fallbacks working on abstract arrays.

[stevengj]

@lindahua, I agree that as long as the fallback has the expected complexity and is correct, there is no reason to restrict it to cases where the constant factor is bad. Subtypes are responsible for providing specialized methods where needed, if possible.

It looks like generic_matmatmul calls copy_transpose!, which currently works only for strided matrices. Actually, it is worse than that: the current implementation in array.jl seems to assume that all strided matrices are column-major, which is now incorrect.

[JeffBezanson]

Overall this is a good change but I don't find the word "regular" very evocative.

[stevengj]

I can always change it back to DenseArray... I don't really care that much.

[mbauman]

It's really just a StridedArray, no? But that's the currently a typealias to include dense SubArrays in a Union. Perhaps it should go back to DenseArray for now and then if/when ArrayViews lands, that level of the hierarchy can be removed since abstract ArrayView{T,N,M} <: DenseArray{T,N} (here). At that point DenseArray could become StridedArray.

[stevengj]

I restored the status of BitArray and SharedArray as subtypes of DenseArray. I made generic_matmul work for arbitrary AbstractArray types (fixing the column-major assumption in copy! and copy_transpose!), but a whole bunch of elementwise operations in array.jl are defined only for StridedArray, and changing them to work for AbstractArray caused all hell to break loose. Lots of method ambiguities due to conflicts with Ranges definitions, for example. So I punted.

Since they don't define strides, leaving them as DenseArray subtypes should be mostly innocuous; the worst that should happen is that a MethodError will be thrown if they are passed somewhere that isn't expecting them (as opposed to crashes...since any code that performs a pointer conversion should also be looking at the strides).

[stevengj]

(Rebased.)