Backslash with SparseMatrix falls back to the generic backslash method

[KristofferC]

This means that cholmod is actually never used even if the matrix is pos def for backslash.

julia> typeof(A)
SparseMatrixCSC{Float64,Int64}

julia> isposdef(A)
true

julia> typeof(x)
Array{Float64,1}

julia> @which A\x
\(A::AbstractArray{T<:Any,2}, B::Union{AbstractArray{T<:Any,1},AbstractArray{T<:Any,2}}) at linalg/generic.jl:333

julia> @time A \ x; # Uses LU factorization because the generic factorize uses this for square matrices
 13.809494 seconds (87 allocations: 1.744 GB, 0.97% gc time)

julia> @time factorize(A) \ x; # Uses Cholesky factorization 
  3.442693 seconds (79 allocations: 648.282 MB, 0.20% gc time)

Shouldn't backslash for sparse matrices use the factorize methods defined at https://github.com/JuliaLang/julia/blob/master/base/sparse/linalg.jl#L859?

cc @andreasnoack

[andreasnoack]

I'll have to look a little more into this. I consolidated some of the \ logic at some point and maybe something unintentional happened.

[andreasnoack]

I think it was an unanticipated consequence of 87e26c5. We should probably have what you suggested so it would be great if you could prepare a PR.

[KristofferC]

I tried the easy way with just \(A, x) = factorize(A) \ x but it fails on the @inferred test in spqr.jl. Julia infers Any from factorize and that of course propagates to \.

[andreasnoack]

..and suddenly I remember the issue and the reason why I added the @inferred test. We might have to add a \ method that is almost identical to factorize.

[tkelman]

@Sacha0 any chance you could take a look at this one? would be much-appreciated!

[Sacha0]

any chance you could take a look at this one?

Shall do!

[Sacha0]

any chance you could take a look at this one?

Shall do!

@tkelman I should be able to spend a decent fraction of the coming week on things Julia. So if there are similar RC-freeze-related tasks in need of attention, please don't hesitate to ask. Best!

[JaredCrean2]

I don't think I can add labels, but if any issue deserves the needs-a-test label, this one does.

[KristofferC]

#16979 includes a test, albeit brittle

[JaredCrean2]

Cool. Dispatch tests like that are important.

[tkelman]

We just took it off the milestone, but maybe you'd be able to make a little bit of headway on #13171 while Andreas is overloaded with JuliaCon organizing?

[KristofferC]

Took what off milestone?

[Sacha0]

Close with benefit of #16979 and open a new issue to track #16979 (comment) / #16979 (comment), or leave this issue open to track those points?

[KristofferC]

Let's close since the actual problem is fixed. Can have another issue for the style "problem".