Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdlib/SparseArrays: fix scalar setindex! for vector eltype #29331

Merged
merged 1 commit into from
Dec 19, 2018

Conversation

stev47
Copy link
Contributor

@stev47 stev47 commented Sep 23, 2018

fixes #29034

There might be more elegant solutions, I'm open to suggestions.

@stev47
Copy link
Contributor Author

stev47 commented Sep 23, 2018

Appveyor failure seems unrelated

@ViralBShah
Copy link
Member

Can we merge this?

@stev47
Copy link
Contributor Author

stev47 commented Dec 19, 2018

Just rebased the branch: to me the appveyor failure does not seem to be related

@ViralBShah ViralBShah added the sparse Sparse arrays label Dec 19, 2018
@@ -2565,7 +2569,8 @@ end
_to_same_csc(::SparseMatrixCSC{Tv, Ti}, V::AbstractMatrix, I...) where {Tv,Ti} = convert(SparseMatrixCSC{Tv,Ti}, V)
_to_same_csc(::SparseMatrixCSC{Tv, Ti}, V::AbstractVector, I...) where {Tv,Ti} = convert(SparseMatrixCSC{Tv,Ti}, reshape(V, map(length, I)))

setindex!(A::SparseMatrixCSC{Tv}, B::AbstractVecOrMat, I::Integer, J::Integer) where {Tv} = setindex!(A, convert(Tv, B), I, J)
setindex!(A::SparseMatrixCSC{Tv}, B::AbstractVecOrMat, I::Integer, J::Integer) where {Tv} = _setindex_scalar!(A, B, I, J)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping we might be able to remove this method entirely, but it looks like it's disambiguating against the method that immediately follows. Using a helper function like this is a good solution.

@ViralBShah ViralBShah merged commit 99efc91 into JuliaLang:master Dec 19, 2018
@StefanKarpinski StefanKarpinski added backport 1.1 triage This should be discussed on a triage call and removed backport 1.1 labels Jan 31, 2019
@mortenpi
Copy link
Contributor

Should this be backported to 1.0 too? The issue is also present in 1.0.3.

@mbauman
Copy link
Member

mbauman commented Feb 20, 2019

Yes, I'd support backporting it — it's a fairly major bugfix.

@mschauer
Copy link
Contributor

mschauer commented May 2, 2019

Ping. I run into this, I believe on Julia Version 1.1.0:

Compute guiding term:
ERROR: LoadError: StackOverflowError:
Stacktrace:
 [1] setindex!(::SparseMatrixCSC{SArray{Tuple{2,2},Float64,2,4},Int64}, ::SArray{Tuple{2,2},Float64,2,4}, ::Int64, ::Int64) at /Users/osx/buildbot/slave/package_osx64/build/usr/share/julia/stdlib/v1.1/SparseArrays/src/sparsematrix.jl:2568 (repeats 80000 times)

@stev47
Copy link
Contributor Author

stev47 commented May 6, 2019

probably because this pull request has not yet been backported

@KristofferC KristofferC removed the triage This should be discussed on a triage call label Aug 26, 2019
@KristofferC KristofferC mentioned this pull request Aug 26, 2019
55 tasks
KristofferC pushed a commit that referenced this pull request Aug 26, 2019
KristofferC pushed a commit that referenced this pull request Aug 27, 2019
@KristofferC KristofferC added the bugfix This change fixes an existing bug label Sep 9, 2019
KristofferC pushed a commit that referenced this pull request Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug sparse Sparse arrays
Projects
None yet
7 participants