Julia: `vec` for sparse arrays should return a view, not a copy

[timholy]

vec is a specialized form of reshape, and for julia 0.5 we went to some effort to ensure that reshape(A, shp) predictably returned a view (not a copy) of an arbitrary AbstractArray A. However, vec has a specialized implementation for sparse arrays that violates this behavior.

[lbollar]

I was wondering where the offending code for this is? I see

vec(x::AbstractSparseVector) = x

in sparsevector.jl, is this the problem or is it somewhere else?

[Sacha0]

If I understand @timholy correctly, his concern is for the SparseMatrixCSC vec method (rather than that for AbstractSparseVector, which returns a view as you show above). (In case you are unfamiliar, the methods function lists the methods (and source locations) for a given function:

julia> methods(vec)
# 4 methods for generic function "vec":
vec(S::SparseMatrixCSC) in Base.SparseArrays at sparse/sparsematrix.jl:371
vec(x::AbstractSparseArray{Tv,Ti,1} where Ti where Tv) in Base.SparseArrays at sparse/sparsevector.jl:816
vec(a::AbstractArray{T,1} where T) in Base at abstractarraymath.jl:34
vec(a::AbstractArray) in Base at abstractarraymath.jl:33

Hence at base/sparse/sparsematrix.jl:370-371 we find the SparseMatrixCSC method,

# Note that unlike `vec` for arrays, this does not share data
vec(S::SparseMatrixCSC) = S[:]

which returns a copy rather than a view.) Best!

(Tangentially, the AbstractSparseVector method seems redundant with the AbstractVector method and perhaps could be removed?)

[martinholters]

So just remove the specializations for SparseMatrixCSC and AbstractSparseVector?

[timholy]

I think it would be acceptable to return a SparseVector. You'd have to create a new nzind vector, but from the standpoint of API consistency I think the only thing that matters is nzval, and presumably that can be shared.

[martinholters]

Ahem, no, you could set values to a non-zero that previously were zero/not stored. That could not be reflected by just sharing nzval, could it?

[mbauman]

Right — changing the stored structure of the vector would end up corrupting the original CSC matrix. A new type could propagate those changes back, but it'd similarly have trouble keeping up with mutations in the matrix. Once you support that, you've effectively created a ReshapedArray.

[timholy]

That's interesting. One could return a ReadOnlySparseVector, but that has its own issues. Removing the specializations seems like the most expedient (and likely, best) choice.

[martinholters]

One could return a ReadOnlySparseVector

... But would still need to prevent the reshaped SparseMatrixCSC from changing its structure.

[mkborregaard]

Would this not require the views to be fast? Currently views of sparse matrices seem slow https://github.com/JuliaLang/julia/issues/21796

[StefanKarpinski]

A decision needs to be made here...

[timholy]

Would be pretty satisfyingly solvable with something along the lines of https://github.com/timholy/ArrayIteration.jl, if that were actually finished.

In the absence of something better, we need to turn these specialized implementations into methods of copy(v::ReshapedArray{<:SparseMatrixCSC}).

[StefanKarpinski]

This should consistently return a view, so the action here is to make vec(s) for a sparse array return a view.