Added sparse_factors function

[sglyon]

Hi @timholy this is the spec builder function from over at Interpolations.jl

I've opened a PR to give you a chance to give feedback, especially regarding the name and whether or not to export the function.

Thanks!

[timholy]

The code is fine---it's not high-performance, but this function doesn't need to be, if we don't export it, because you're using it only for small problems.

The help text is a little hard to follow: what is args? (it's clear if you read the code, but...). You might want to mention that rowspec, valspec, and colspec are all sparse matrices.

[timholy]

If we don't export it, the name isn't crucial, but somehow the name seems like it could be a little better. woodbury probably doesn't need to be in the name anymore, since this is what the package is about. What does the specs part of the name mean?

Maybe call it sparse_factors, in honor of the sparse function and the output?

[sglyon]

Thanks for the comments. I definitely agree that woodbury should be taken out of the name.

The specs part was just inherited from the Interpolations code where we were calling these matrices rowspec, colspec, valspec.

I like the factor idea much better. Right now the valspec matrix is dense, but it really just needs to be a Diagonal. If we made that change, would it make sense to use sparse_factors still? I think so as diagonal is just a special case of sparse in my mind

[timholy]

Sure, that's still sparse, even if not a SparseMatrixCSC (which is an implementation detail, in my opinion).

[sglyon]

So I tried allowing the vals part be a Diagonal, but there is no method *(::SparseMatrixCSC, ::Diagonal) (ref JuliaLang/julia#14416)

For now it is also just a SparseMatrixCSC

[timholy]

Sorry I didn't notice this was ready. Thanks!

See also #6.

[sglyon]

No problem, thanks!

[tkelman]

this needs Compat