Remove methods for creating "sparse vectors"

[lindahua]

Currently, we have a bunch of methods that claim to construct sparse vectors but instead produce a sparse matrix (of type SparseMatrixCSC).

Here is a (probably incomplete) list of such functions:

vec(::SparseMatrixCSC) 

sparsevec( ... )

sparse(::Vector)

getindex(::SparseMatrixCSC, :, i::Integer)

Related to #8718. At least, we should deprecate them.

[tkelman]

It would likely be pretty disruptive to completely remove the ability to do getindex(::SparseMatrixCSC, :, i::Integer). What we're doing right now for that is inconsistent with the dense case though, and we should find a way to fix it.

julia> A = rand(5,5)
5x5 Array{Float64,2}:
 0.748413  0.557463  0.0411035  0.311134  0.427405
 0.672685  0.375904  0.353719   0.858432  0.849331
 0.668751  0.150029  0.400771   0.536376  0.0863048
 0.838338  0.443399  0.839185   0.391709  0.586837
 0.891324  0.747012  0.182979   0.397711  0.985923

julia> As = sprand(5,5,0.3)
5x5 sparse matrix with 7 Float64 entries:
        [2, 1]  =  0.646653
        [2, 2]  =  0.70528
        [3, 3]  =  0.793324
        [2, 4]  =  0.397927
        [5, 4]  =  0.442372
        [2, 5]  =  0.814845
        [4, 5]  =  0.162152

julia> A[:, 3]
5-element Array{Float64,1}:
 0.0411035
 0.353719
 0.400771
 0.839185
 0.182979

julia> As[:, 3]
5x1 sparse matrix with 1 Float64 entries:
        [3, 1]  =  0.793324

Given that there now exists a SparseVectors.jl package there is will be? at least someplace else to find that method that works.

[lindahua]

If it is considered to be too disruptive, we can consider deprecating them, and pointing people who need those functions to the SparseVectors package.

[tkelman]

Deprecation without a direct in-base replacement is an interesting idea. Let's try it.

And looking more closely I see you have a view to get a column of a SparseMatrixCSC but you don't have a getindex method to return a copy at this time. Is there any way right now to make getindex on a Base type call a non-base method?

[lindahua]

What about the following way:

In Base, we have

function getindex(A::SparseMatrixCSC, ::Colon, i::Real)
    depwarn("A[:, i] for sparse matrix is deprecated in Base. Please import the SparseVectors package for a corrected implementation")
    # still does what it is doing now
end

In SparseVectors package, we have

Base.getindex(A::SparseMatrixCSC, ::Colon, i::Integer) = # correct implementation that returns a SparseVector

So if SparseVectors is imported, it just works with the corrected behavior (without warning). Otherwise, it still works, but with the current behavior, and the caller will see a warning.

[tkelman]

Clever. Related to #10154, but it sounds like it could work.

[nalimilan]

It sounds weird that you wouldn't be able to index a sparse matrix without installing and loading an additional package. This also means that the behavior of sparse matrix indexing could depend on which one of competing sparse vector packages is loaded?

It would make more sense to me to link the matrix and vector sparse formats in a common package/module, since when indexing the former you get the latter. Alternative implementations would also provide both in order to offer a predictable indexing behaviour -- which is probably also more efficient since a specific sparse matrix format might be associated with a specific efficient structure for sparse vectors.

So in the terms of #11324 this is a vote for (B).

[tkelman]

Agreed, this now looks to me like a convoluted and fragile way to avoid fixing something that we know is broken.

[ViralBShah]

I really want to just fix it, and change the existing broken behaviour ASAP.

[tkelman]

Let's close this option then. @lindahua can you work on a PR to bring in the vector type and basic arithmetic functionality, and fix the broken vector/getindex methods mentioned here? I think it's best to leave the advanced views and unsafe references stuff out in the package for now.

[ViralBShah]

Yes, leave views and unsafe references in the package - perhaps can become part of Arrayviews.jl or NumericExtensions.jl.

[tkelman]

Or stay where it is, since the package will still need to exist to provide support for 0.3

[ViralBShah]

Good point.