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

Inconsistency in descending transpose between sparse and dense arrays #28369

Closed
jlapeyre opened this issue Jul 31, 2018 · 4 comments
Closed

Inconsistency in descending transpose between sparse and dense arrays #28369

jlapeyre opened this issue Jul 31, 2018 · 4 comments
Labels
bug Indicates an unexpected problem or unintended behavior sparse Sparse arrays

Comments

@jlapeyre
Copy link
Contributor

using LinearAlgebra
using SparseArrays
A = [1 0 ; 1 0]

julia> M = reshape([A, A, A, A], (2,2))
2×2 Array{Array{Int64,2},2}:
 [1 0; 1 0]  [1 0; 1 0]
 [1 0; 1 0]  [1 0; 1 0]

julia> copy(transpose(M))
2×2 Array{Array{Int64,2},2}:
 [1 1; 0 0]  [1 1; 0 0]
 [1 1; 0 0]  [1 1; 0 0]

julia> Array(copy(transpose(sparse(M))))
2×2 Array{Array{Int64,2},2}:
 [1 0; 1 0]  [1 0; 1 0]
 [1 0; 1 0]  [1 0; 1 0]

Ideally, I expect sparse and dense objects to represent the same thing, but to employ different representations internally. I convert between them only to take advantage of differences in algorithmic efficiency.

I guess there is no perfect solution to this kind of thing. Computer-language types and mathematical-object types (sets) are far from congruent. E.g., in math, it is common to interpret objects in one section of a manuscript or book in a particular way without worrying about breaking examples in another section. Even the most flexible computer-language type system is more rigid.

@andreasnoack
Copy link
Member

andreasnoack commented Jul 31, 2018

It's a bug. Maybe easier to see in

julia> transpose(sparse(M))[1,1]
2×2 Transpose{Int64,Array{Int64,2}}:
 1  1
 0  0

julia> copy(transpose(sparse(M)))[1,1]
2×2 Array{Int64,2}:
 1  0
 1  0

I guess copy(::Transpose/Adjoint) needs to be adjusted somehow.

@andreasnoack andreasnoack added bug Indicates an unexpected problem or unintended behavior sparse Sparse arrays labels Jul 31, 2018
@andreasnoack
Copy link
Member

Indeed the two definitions

Base.copy(A::Adjoint{<:Any,<:SparseMatrixCSC}) = ftranspose(A.parent, conj)
Base.copy(A::Transpose{<:Any,<:SparseMatrixCSC}) = ftranspose(A.parent, identity)
are only correct for scalar element types.

@fredrikekre
Copy link
Member

IIRC the assumption of scalar elements are used quite a lot in the sparse code.

@jlapeyre
Copy link
Contributor Author

Not sure how robust this is, but

julia> Base.copy(A::Transpose{<:Any,<:SparseMatrixCSC}) = 
     SparseArrays.ftranspose(A.parent,  x -> copy(transpose(x)));

julia> S = sparse(M);

julia> copy(transpose(S))[1,1]
2×2 Array{Int64,2}:
 1  1
 0  0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior sparse Sparse arrays
Projects
None yet
Development

No branches or pull requests

3 participants