added hcat/vcat/hvcat methods for UniformScaling

[stevengj]

It is nice to be able to construct matrices with identity components using I, where the size of the identity is inferred from the other pieces of the matrix. e.g. with this PR you can now do:

julia> [rand(3,3) I; 2I 3I; 4I]
12×6 Array{Float64,2}:
 0.0206989  0.703627   0.582238  1.0  0.0  0.0
 0.993276   0.0728296  0.900664  0.0  1.0  0.0
 0.424733   0.578488   0.081822  0.0  0.0  1.0
 2.0        0.0        0.0       3.0  0.0  0.0
 0.0        2.0        0.0       0.0  3.0  0.0
 0.0        0.0        2.0       0.0  0.0  3.0
 4.0        0.0        0.0       0.0  0.0  0.0
 0.0        4.0        0.0       0.0  0.0  0.0
 0.0        0.0        4.0       0.0  0.0  0.0
 0.0        0.0        0.0       4.0  0.0  0.0
 0.0        0.0        0.0       0.0  4.0  0.0
 0.0        0.0        0.0       0.0  0.0  4.0

For sparse matrices, it concatenates a sparse identity efficiently. I also added a sparse(I, m,n) constructor since this seemed generally useful.

This change is technically breaking because hcat(I, I) now throws an error (because it can't figure out the size of the resulting matrix), whereas before it returned a 1×2 Array{UniformScaling{Int64},2}. I doubt this will affect many people.

[StefanKarpinski]

I like it, but of course, this leads to wanting Ones and Zeros objects that behave similarly.

[stevengj]

@StefanKarpinski, you can get a zeros object just by 0I. I'm skeptical of the need for a Ones object (corresponding to an implicit-size ones?) in this sort of matrix construction.

[stevengj]

PS. I would have used @inferred in the tests, but ran into #19304.

[StefanKarpinski]

If UniformScaling were generalized to have a base value and a diagonal value, then we could make things like 0*I+1 work. I'm not sure if that's something we should do, but it's worth considering.

[andreasnoack]

Great. I think this will be really useful.

[stevengj]

Matrices full of constants just aren't that useful in linear algebra, so I doubt it is worth it. (This is similar to the discussion in #14963 about whether we should implement "sparse" matrices with a nonzero "default" value.)

[StefanKarpinski]

I dunno, I've written a lot of code concatenating ones(m,n) with other matrices. Often m or n is 1 but not always. I actually think that having sparse matrices with non-zero default is a good idea since it makes many operations on sparse matrices closed. The main drawback is the complexity of the implementation. In this case the implementation is pretty simple.

[stevengj]

Getting

    FAILURE
Error in testset spawn:
Test Failed
  Expression: false

on Linux, which seems unrelated. Is there a problem with Travis?

[stevengj]

General low-rank updates would be useful, but they vastly complicate the implementation; low-rank matrices are really their own data structure (e.g. for BFGS updates). Adding a constant everywhere is just a very special case of a rank-1 update, and in my opinion is not worth the trouble (every single sparse-matrix routine everywhere would have to be updated, and unexpectedly so — people writing "sparse" code would not expect this definition of "sparse"), and SuiteSparse would be a hassle. In any case, this seems like something to be discussed in its own issue.

[stevengj]

Restarting CI now that #19132 is closed.

[stevengj]

Yay, CI passes again. Okay to merge, since it seems that supporting this functionality is uncontroversial?

[tkelman]

should have run nanosoldier here, make sure this doesn't slow down other concatenation cases

[stevengj]

How could it? The new code is only called when you have a UniformScaling argument.

[tkelman]

it's called on a union, and a similar change with sparse matrices caused a big regression several months ago

[andreasnoack]

I'm not sure if you two noticed 832ab88#commitcomment-19855129.

[stevengj]

Thanks @andreasnoack. Also, @tkelman, in this case it is defined for Union{AbstractVecOrMat,UniformScaling}... arguments, but there is already an hcat method (etc.) defined for AbstractVecOrMat..., so as far as I can tell it is not possible to call the new hcat method unless you pass a UniformScaling argument. If you pass only arrays, it will always call the old AbstractVecOrMat... method.

[tkelman]

does look like this caused a stack overflow in ImageFiltering.jl's tests http://pkg.julialang.org/logs/ImageFiltering_0.6.log

[stevengj]

Ah, looks like we need an hvcat(::Tuple{Vararg{Int}}, ::AbstractVecOrMat...) method to disambiguate.

[stevengj]

A similar example that overflows (via a dispatch loop) is [(1:2) zeros(2,2); ones(3,3)]