support general backends for BlockArray

[dlfivefifty]

This allows general backends for BlockArray. For example:

julia> BlockArrays._BlockArray(Fill([1,2],5), Fill(2,5))
10-element BlockArray{Int64,1,Fill{Array{Int64,1},1,Tuple{Base.OneTo{Int64}}}}:
 1
 2
 ─
 1
 2
 ─
 1
 2
 ─
 1
 2
 ─
 1
 2

[codecov-io]

Codecov Report

Merging #76 into master will decrease coverage by 4.08%.
The diff coverage is 97.5%.

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
- Coverage    80.1%   76.02%   -4.09%     
==========================================
  Files          12       12              
  Lines         578      588      +10     
==========================================
- Hits          463      447      -16     
- Misses        115      141      +26

Impacted Files	Coverage Δ
src/blockbroadcast.jl	71.18% <ø> (-7.15%)	⬇️
src/views.jl	56.52% <ø> (-18.48%)	⬇️
src/blockarrayinterface.jl	42.85% <ø> (-28.58%)	⬇️
src/blockindices.jl	50% <ø> (ø)	⬆️
src/show.jl	88.67% <100%> (+3.31%)	⬆️
src/pseudo_blockarray.jl	85.91% <100%> (ø)	⬆️
src/blocksizes.jl	86.11% <100%> (+6.65%)	⬆️
src/blockarray.jl	91.81% <100%> (-2.73%)	⬇️
src/abstractblockarray.jl	86.56% <75%> (-2.5%)	⬇️
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1006b8...af1972a. Read the comment docs.

[fredrikekre]

Could this functionality be included in some other type under the BlockArrayInterface? It is rather annoying to introduce extra type parameters for BlockArray.

[dlfivefifty]

This is to support special blocksizes with extra structure (for example, constant block sizes #26). I don't see how else to do it apart from adding another templated variable.

Type-aliases can give the current behaviour if needed:

const DefaultBlockSizes{N} = BlockSizes{N,Vector{Int}}
const DefaultBlockArray{T, N, R} = BlockArray{T,N,R,DefaultBlockSizes{N}}

[fredrikekre]

I think that BlockArray should just be the standard implementation without much extra functionality, just like Array is the standard implementation of the abstract array interface.

[dlfivefifty]

It's a modifier type, that takes in something else and reinterprets it. It's more like Diagonal or SubArray, which support many backends.

[tkf]

A slight variant to the type-alias approach would be to define struct GenericBlockArray (say) etc. and create type-aliases BlockArray etc. It's nicer in terms of backward-compatibility. A down-side is that BlockArray type would be printed as GenericBlockArray (but defining show(::IO, ::Type{<:BlockArray}) may not be super crazy; see JuliaLang/julia#31149).

(Side notes: I had a similar pull request for SparseMatrixCSC JuliaLang/julia#30173. There, I introduced GenericMatrixCSC and turn SparseMatrixCSC to a type-alias. I think it worked quite well in terms of backward-compatibility.)

I guess another approach would be to define getter/setter for AbstractBlockArray etc. as optional interfaces and turn all method definitions using the abstract types. But it would increase API surface of the abstract types without knowing the actual usecases. So, I think current PR is a better way to go.

[dlfivefifty]

Sure that's fine. How about Mortar instead of GenericBlockArray?

[fredrikekre]

I just don't understand the need to push every usecase into the same struct, this makes things unnecessary complicated IMO. Most users will never use this and would be better off with just (e.g. even removing the R parameter)

struct BlockArray{T, N} <: AbstractBlockArray{T, N}
    blocks::Array{Array{T,N}, N}
    block_sizes::BlockSizes{N}
end

Why can't this functionality just be included in a

struct GeneralBlockArray{T, N, R, S, ...} <: AbstractBlockArray{T, N}
    ...
end

?

[dlfivefifty]

Because the current BlockArray is essentially useless for high-performance use cases. We want to support parallelised backends. Why have an extra data structure that does the exact same thing as the more general data structure with only very specific, low performance, use cases?

[dlfivefifty]

And the exact same logic applies to Diagonal, by the way. What you are advocating is that every type has two versions: one for Array and one for other types. That is just a huge amount of unnecessary repeated code.

[tkf]

How about Mortar instead of GenericBlockArray?

I think you may also need GenericBlockSizes and maybe other similar types? So, I thought something like Generic*-prefix would be nice for creating a coherent API.

[dlfivefifty]

BlockSize is hidden from the users so I’m not sure theres a need for GenericBlockSize

[fredrikekre]

That is just a huge amount of unnecessary repeated code.

Isn't this handled by the AbstractBlockArray interface?

[dlfivefifty]

No AbstractBlockArray is for more general storage like PseudoBlockArray

[tkf]

@dlfivefifty I see. Another argument against this is that BlockArray <: Mortar <: AbstractBlockArray is a bit strange in that Mortar does not have BlockArray in it.

Actually, I think this PR as-is without re-naming using Generic*-prefix makes sense since you can bump the major version in non-stdlib relatively easily.

[fredrikekre]

No AbstractBlockArray is for more general storage like PseudoBlockArray

Why couldn't it be used for a new struct like GeneralBlockArray? If internals are implemented in terms of general methods like getblock etc everything should work.

[dlfivefifty]

You would need an extra abstract type to be able to reuse code. Which makes the code more complex than just doing a typealias as the implementation is identical.

I really don’t see the argument against a typealias as for users of BlockArray everything behaves exactly as it does now with no need for extra abstract and concrete types.

[dlfivefifty]

@dlfivefifty I see. Another argument against this is that BlockArray <: Mortar <: AbstractBlockArray is a bit strange in that Mortar does not have BlockArray in it.

What I like about Mortar is that it has the same simplicity as other "modifier" arrays (Symmetric, Adjoint, Diagonal, ...), but then the name PseudoBlockArray is not so nice... The best I could come up with is Stucco, which is too punny.

[tkf]

What I like about Mortar is that it has the same simplicity as other "modifier" arrays (Symmetric, Adjoint, Diagonal, ...)

I agree that simplicity is appealing. But I think it would be confusing to have both BlockArray and Mortar, especially since their sub-typing relationship is not obvious.

[dlfivefifty]

It looks like only two packages (other than mine) depend on BlockArrays.jl: Stheno.jl, which won't need any changes, and MixedModels.jl, which will need to be updated if we change the templates for BlockArray, but won't if we make it a typealias to GenericBlockArray.

@willtebbutt @dmbates do you have any strong feelings about the proposed change to allow general BlockSizes as a template variable to support, e.g., constant block sizes and symmetric block sizes?

[willtebbutt]

I am in favour of this change. Being able to use a Diagonal matrix to contain BlockArray.blocks is likely to be very helpful for some of my applications. I suspect that this might also be helpful for @eperim - doesn't Invenia currently have a custom implementation of a block diagonal matrix floating around somewhere? This PR would enable you to use BlockArrays, right?

[dlfivefifty]

Note it also supports BlockTridiagonal, though it requires a bit more overloads currently in

https://github.com/JuliaMatrices/BlockBandedMatrices.jl/pull/24/files

(I could move these changes to support BlockTridiagonal here. )

[dlfivefifty]

I think the best solution is to simplify the default output to hind the implementation details from the user:

julia> A = BlockArray{Float64}(undef, 1:3)
3-blocked 6-element BlockArray{Float64,1}:
 2.2619260434e-314
 ─────────────────
 2.3300040286e-314
 2.29625195e-314  
 ─────────────────
 2.330002479e-314 
 2.330002479e-314 
 2.330002479e-314 

This avoids unnecessary extra type names.