Fix #13130 - concatenation of dense and sparse should be sparse.

[pkofod]

Fixes #13130 .

I had to add hcat, vcat, and hvcat for Matrix. If I didn't, it would fall back to the Union{Matrix, SparseMatrixCSC method for [rand(10,10) rand(10, 10)]. There may be cleaner ways to do all of this.

Also, currently [sprand(10, 0.1) sprand(10, 10, 0.1)] returns a dense matrix. Surely, that should be sparse. I havn't implemented [rand(10) sprand(10, 10, 0.1)] yet either.

I guess it is still to be decided if this is the wanted behaviour, but I just wanted to make a PR to mirror my issue.

By the way, I am unsure what happened with the first two commits mentioned at the issue. They were supposed to be to my fork, but it doesn't say "to pkofod/julia". Is that because I've deleted that fork since?

[pkofod]

It couldn't build, as I had added the *cat methods in the wrong file, but it builds locally, so I expect it to build on travis as well - although I might have killed some tests.

[pkofod]

Test failure seems to be due to #15285.

I havn't done anything about the fact that [spvector spmatrix] doesn't exist, as it should probably be a different PR.

[tkelman]

Could this be done without having to build up and save temporary S copies for all of [X[1] X[2]] and [X[1] X[2] X[3]] and [X[1] X[2] X[3] X[4]] etc?

[pkofod]

Do you mean like we do when cat'ing sparse matrices?

[pkofod]

I tried to address your concern @tkelman . However, my hcat and vcat are not type stable, so hvcat fails to setindex into the SparseMatrixCSC array, see https://gist.github.com/pkofod/0e8fa1bbadebca9127d68fa5ec5dd0b0 .

I would appreciate pointers as to what I am doing wrong here.

[tkelman]

MethodError: Cannot convert an object of type Array{Float64,2} to an object of type SparseMatrixCSC{Tv,Ti<:Integer}

Seems like we could have a go at defining that conversion method?

[pkofod]

Ah, now I get it! Obviously, this is going to fail if an entire row is dense. Thanks :)

[pkofod]

So I pushed another version, which simply does

        tmp_rows[i] = sparse(hcat(X[(1 : rows[i]) + k]...))

I figured that would be fine, as I thought

sparse(S::SparseMatrixCSC) = S

but apparently

sparse(S::SparseMatrixCSC) = copy(S)

I just can't quite figure out how to define the convert method, as the Array I'm indexing into is of SparseMatrixCSC without any parametrization. I thought

convert(::SparseMatrixCSC, M::Matrix) = sparse(M)

Would work, but apparently not.

[nalimilan]

I just can't quite figure out how to define the convert method, as the Array I'm indexing into is of SparseMatrixCSC without any parametrization. I thought

convert(::SparseMatrixCSC, M::Matrix) = sparse(M)

Would work, but apparently not.

You need convert(::Type{SparseMatrixCSC}, M::Matrix) = sparse(M)

[pkofod]

You need convert(::Type{SparseMatrixCSC}, M::Matrix) = sparse(M)

Of course, thanks.

[pkofod]

travis-ci timed out on osx

For what it is worth, sparse.jl actually ran successfully on OSX before the time out :)

[pkofod]

This time around it was the x86_64 linux build that failed, but that was due to an apt-get error. If it needs to be run, then maybe someone can simply restart that build, or do I need to run them all again?

(thanks)

[pkofod]

Doing the hvcat I realized it was using the Array(T, dims...) which was deprecated in v0.4, and has to go in this cycle, right? I figured while I was in /base/sparse I might as well update these. I could do it in all of base, and I happily volunteer to do that, but that is probably for a different PR. For now I just did it in /sparse/, and in a separate commit from the *cat changes.

[pkofod]

What happened in AppVeyor?

[tkelman]

whatever it was, it's not good:

Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
Exception: EXCEPTION_ACCESS_VIOLATION at 0x0 -- unknown function (ip: 0000000000000000)
while loading no file, in expression starting on line 0
unknown function (ip: FFFFFFFFFFFFFFFF)
Allocations: 904230 (Pool: 903129; Big: 1101); GC: 1

Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
Exception: EXCEPTION_ACCESS_VIOLATION at 0x0 -- unknown function (ip: 0000000000000000)
while loading no file, in expression starting on line 0
unknown function (ip: FFFFFFFFFFFFFFFF)
Allocations: 904230 (Pool: 903129; Big: 1101); GC: 1

though I'd be surprised if it were due to your changes, likely a preexisting bug somewhere

[pkofod]

@tkelman any chance I could get you to restart the two failing runs? (mac on travis and 64bit on AV)

[nalimilan]

Done.

[pkofod]

Thanks. I see travis succeeded, but AV didn't run.

[nalimilan]

Ah, now it says the PR has conflicts and thus cannot run.

[pkofod]

Oh, so I should rebase and push?

Edit: Some tests had been added to sparse.jl. Rebased.

[pkofod]

CI, a love-hate relationship

One was the DictChannel bug, one was a time-out.

[nalimilan]

OS X failure is expected at the moment, I've restarted the other.

[pkofod]

If there are any comments at this stage, I'd gladly take them.

[pkofod]

Sorry to bother the subscribed people, I'm just curious. Is this waiting for a decision or something similar? I'd just prefer not to rebase again before I know what going on, as I've rebased a few times just for the sake of rebasing :)

[tkelman]

I think we just keep forgetting to merge before new conflicts happen, sorry. I don't see anything objectionable here and agree with the change myself.

[pkofod]

I think we just keep forgetting to merge before new conflicts happen, sorry.

No worries! I know these are busy times.

[tkelman]

oh yeah I guess we could technically use a generator here now, though I'm not sure if it would make a noticeable difference

[pkofod]

I am not quite sure I know what you mean.

[tkelman]

Generators are new syntax that's like comprehensions but creating an iterator instead of an array. Though it looks like hcat'ing a comprehension generator without a splat doesn't behave quite like I thought it would.