diagm for non-square matrices

[stevengj]

This PR adds a size=(m,n) keyword argument optional m,n initial arguments to diagm and spdiagm so that they can be used to create non-square matrices.

I mainly care about this capability for spdiagm, which was removed in #23757: I often create non-square matrices as a part of creating finite-difference operators via Kronecker products, and the current workaround is to call SparseArrays.spdiagm_internal. But if spdiagm gets this feature than diagm clearly should as well.

Instead of size=(m,n) argument, I suppose it could accept nrows and ncols or similar. I don't have strong feelings, but size=(m,n) seemed cleaner and more comprehensible to me, since if you call diagm(...; nrows=N) it is very unclear what to expect for ncols (should it =nrows or should it be the minimum number of columns for the given diagonals?). It could also have m,n as the first positional arguments instead of as a keyword, I suppose.

[stevengj]

I suppose this is too late for 1.2, so I should wait until 1.3-dev to add a NEWS entry, I guess?

[ararslan]

Yeah, we'll be branching for 1.2 shortly.

[andreasnoack]

I'm all for the functionality here. It's a little unfortunate that we have to use a keyword argument for the size when we use either positional arguments everywhere else for specifying the size. Can't really come up with a better suggestion right now, though.

[StefanKarpinski]

It's a little unfortunate that we have to use a keyword argument for the size when we use either positional arguments everywhere else for specifying the size. Can't really come up with a better suggestion right now, though.

Why not as a positional argument? This is currently a method error:

julia> diagm(rand(3), (3, 4))
ERROR: MethodError: no method matching diagm(::Array{Float64,1}, ::Tuple{Int64,Int64})

What should this do if you call diagm(rand(3), (4, 5)) though? Just fill in as much of the diagonal as you have? Or raise an error if one of the dimensions is not equal to the diagonal length? What about diagm(rand(4), (2, 3))? Should it just ignore all but the first two diagonal values?

[stevengj]

@StefanKarpinski, I think it's pretty clear it should pad with zeros, and raise an error if the (m,n) size is too small to hold the requested diagonal. (That's the behavior in the current PR.)

If we use a positional argument, it would be more consistent with our other APIs to pass m,n as separate arguments rather than as a tuple. Do we want them to be the first or the last arguments?

[fredrikekre]

Do we want them to be the first or the last arguments?

IMO last, similar to e.g. Matrix{Float64}(undef, m, n) etc, but does that work? ... is not allowed for non-final arguments.

[stevengj]

The implementation would certainly be easier if m,n were the initial arguments; I don't know of a way to put it last unless there is some trick with Vararg?. I agree that it would be closer to something like fill(x, dims) to put the size last.

[stevengj]

Another possibility would be a tight=true keyword that makes the matrix the minimal size to hold the requested diagonals. For example, you would have:

julia> diagm(0=>[1,2,3], 1=>[4,5,6], tight=true)
3×4 Array{Int64,2}:
 1  4  0  0
 0  2  5  0
 0  0  3  6

Even if we allow a general m,n option (zero-padding might be useful for something like the Σ matrix in the full SVD), a tight option would be nice for this common case so that you don't have to specify an m,n that is redundant with information already in the diagonals.

(Not sure if there is a better word than tight here.)

[stevengj]

Updated to use m,n optional initial arguments rather than a keyword argument.

[stevengj]

AppVeyor i686 "Error in testset Profile" seems unrelated (#29880).