make sparse arrays print more consistently, also add some doctests #20488

KristofferC · 2017-02-07T09:18:43Z

Currently, the printout is a bit all over the place for sparse arrays:

julia> sprand(1,1,1.0)
1×1 sparse matrix with 1 Float64 stored entries:
	[1, 1]  =  0.905703

julia> rand(1,1)
1×1 Array{Float64,2}:
 0.238367

julia> sprand(1, 1.0)
Sparse vector of length 1 with 1 Float64 stored entries:
  [1]  =  0.580738


julia> rand(1)
1-element Array{Float64,1}:
 0.0552593

You can for example not see what type the indices are stored in. This PR makes it more consistent:

julia> sprand(1,1,1.0)
1×1 SparseMatrixCSC{Float64,Int64} with 1 stored entry:
  [1, 1]  =  0.241341

julia> rand(1,1)
1×1 Array{Float64,2}:
 0.278641

julia> sprand(1, 1.0)
1-element SparseVector{Float64,Int64} with 1 stored entry:
  [1]  =  0.873895

julia> rand(1)
1-element Array{Float64,1}:
 0.0355468

KristofferC · 2017-02-07T12:07:16Z

I discussed a bit with @fredrikekre and will update soon with the new proposal.

fredrikekre · 2017-02-07T12:09:33Z

I propose this instead (consistent with Array and many other outputs):

julia> sprand(2, 2, 1.0)
2×2 SparseMatrixCSC{Float64,Int64} with 4 stored entries:
  [1, 1]  =  0.0506357
  [2, 1]  =  0.143032
  [1, 2]  =  0.932698
  [2, 2]  =  0.906617

julia> sprand(2, 1.0)
2-element SparseVector{Float64,Int64} with 2 stored entries:
  [1]  =  0.194626
  [2]  =  0.264848

KristofferC · 2017-02-07T12:39:41Z

Updated top post.

fredrikekre · 2017-02-07T12:44:27Z

base/sparse/sparsevector.jl

+  [1]  =  1.0
+
+julia> Base.SparseArrays.dropstored!(x, 2)
+3-element SparseVector{Float64,Int64} with 1 stored entries:


Why does this not show as entry?

It does, just messed up with the doctesting

KristofferC · 2017-02-07T17:11:39Z

 Got an exception of type MethodError outside of a @test
  MethodError: no method matching isassigned(::Array{Float64,1}, ::Int64)
  Closest candidates are:
    isassigned{T}(::Array{T,N} where N, !Matched::Int32...) at array.jl:67
    isassigned(::AbstractArray, !Matched::Int32...) at abstractarray.jl:213

So is isassigned overtyped since it takes Int... but could as well take Integer...?

Sacha0 · 2017-02-07T23:30:33Z

base/sparse/sparsematrix.jl

@@ -117,7 +117,9 @@ column. In conjunction with [`nonzeros`](@ref) and
 nzrange(S::SparseMatrixCSC, col::Integer) = S.colptr[col]:(S.colptr[col+1]-1)

 function Base.show(io::IO, ::MIME"text/plain", S::SparseMatrixCSC)
-    print(io, S.m, "×", S.n, " sparse matrix with ", nnz(S), " ", eltype(S), " stored entries")
+    xnnz = nnz(S)
+    print(io, S.m, "×", S.n, " ", typeof(S), " with ", nnz(S), " stored ",


Why nnz(S) the first time and xnnz(= nnz(S)) the second?

not addressed

Thanks for reminder. Updated.

Sacha0

Beautiful printing!

tkelman · 2017-02-08T00:46:41Z

base/sparse/sparsematrix.jl

+
+julia> Base.SparseArrays.dropstored!(A, 1, 2); A
+2×2 SparseMatrixCSC{Int64,Int64} with 1 stored entry:
+  [1, 2]  =  1


wasn't this supposed to drop A[1, 2] ? is there an off-by-one in what dropstored! does to the colptr?

Good catch --- dropstored! might be broken?

julia> foo = sprand(4, 4, 0.5) 4×4 sparse matrix with 10 Float64 stored entries: [2, 1] = 0.986522 [1, 2] = 0.331456 [2, 2] = 0.181261 [4, 2] = 0.545928 [1, 3] = 0.864019 [2, 3] = 0.48957 [3, 3] = 0.575748 [4, 3] = 0.509864 [1, 4] = 0.368723 [3, 4] = 0.332495 julia> Base.SparseArrays.dropstored!(foo, 2, 1) 4×4 sparse matrix with 9 Float64 stored entries:Error showing value of type SparseMatrixCSC{Float64,Int64}: ERROR: BoundsError: attempt to access 9-element Array{Int64,1} at index [0] Stacktrace: [1] show(::IOContext{Base.Terminals.TTYTerminal}, ::SparseMatrixCSC{Float64,Int64}) at ./sparse/sparsematrix.jl:147 [2] show(::IOContext{Base.Terminals.TTYTerminal}, ::MIME{Symbol("text/plain")}, ::SparseMatrixCSC{Float64,Int64}) at ./sparse/sparsematrix.jl:122 [3] display(::Base.REPL.REPLDisplay{Base.REPL.LineEditREPL}, ::MIME{Symbol("text/plain")}, ::SparseMatrixCSC{Float64,Int64}) at ./REPL.jl:122 [4] display(::Base.REPL.REPLDisplay{Base.REPL.LineEditREPL}, ::SparseMatrixCSC{Float64,Int64}) at ./REPL.jl:125 [5] display(::SparseMatrixCSC{Float64,Int64}) at ./multimedia.jl:194 julia> foo.colptr 5-element Array{Int64,1}: 0 1 4 8 10

tkelman · 2017-02-08T00:58:49Z

base/sparse/sparsevector.jl


 Create a sparse vector of length `m` where the nonzero indices are keys from
 the dictionary, and the nonzero values are the values from the dictionary.
+
+```jldoctest
+julia> sparsevec(Dict(1 => 3, 2 => 2, 2 => 4))


for a minute I thought "oh that's interesting, sparsevec from a Dict doesn't do combining" but no, the overwriting happens in the Dict constructor so the duplicate key is probably confusing to have here

Thanks, fixed.

KristofferC · 2017-02-09T19:37:45Z

Does anyone understand where this error on 32 bit comes from:

Error in testset sparse/sparse:
Error During Test
  Got an exception of type MethodError outside of a @test
  MethodError: no method matching isassigned(::Array{Float64,1}, ::Int64)
  Closest candidates are:
    isassigned(::Array{T,N} where N, !Matched::Int32...) where T at array.jl:67
    isassigned(::AbstractArray, !Matched::Int32...) at abstractarray.jl:213
  Stacktrace:
   [1] similar(::Type{T} where T, ::Tuple{Base.OneTo{Int32}}) at ./abstractarray.jl:0
   [2] copy!(::AbstractArray, ::Any) at ./abstractarray.jl:0
   [3] inlineable(::Any, ::Any, ::Expr, ::Array{Any,1}, ::Core.Inference.InferenceState) at ./inference.jl:3840
   [4] _setindex!(::Dict{Symbol,Any}, ::Set{Char}, ::Symbol, ::Int32) at ./dict.jl:396
   [5] _setindex!(::Dict{Symbol,Any}, ::Set{Char}, ::Symbol, ::Int32) at ./dict.jl:403
   [6] #show#7(::Bool, ::Function, ::IOContext{Base.AbstractIOBuffer{Array{UInt8,1}}}, ::StackFrame) at ./stacktraces.jl:0
   [7] (::Base.#kw##show_trace_entry)(::Array{Any,1}, ::Base.#show_trace_entry, ::IOContext{Base.AbstractIOBuffer{Array{UInt8,1}}}, ::StackFrame, ::Int32) at ./<missing>:0 (repeats 2 times)
   [8] showerror(::IOContext{Base.AbstractIOBuffer{Array{UInt8,1}}}, ::MethodError) at ./replutil.jl:292
   [9] showerror(::IOContext{Base.AbstractIOBuffer{Array{UInt8,1}}}, ::MethodError) at ./replutil.jl:303
   [10] show_method_candidates(::IOContext{Base.AbstractIOBuffer{Array{UInt8,1}}}, ::MethodError, ::Array{Any,1}) at ./replutil.jl:503

I added a workaround for now: e87c7f2 but I don't understand why this happens just for this case and not for printing any other array types.

KristofferC · 2017-02-10T15:14:35Z

I feel this could be merged if no one has a problem with the e87c7f2 commit.

tkelman · 2017-02-11T09:05:31Z

worth looking into why that's necessary

KristofferC · 2017-02-11T20:17:13Z

The stacktrace for that error (above) seems bonkers.. It's trying to show a the closest candidates for a MethodError. But what would throw a MethodError. And then have the build succeed when I define the isassigned method to work with other integer types?

tkelman · 2017-02-11T20:51:26Z

Backtrace is better with julia-debug. The isassigned is coming from

julia/base/sparse/sparsematrix.jl

Line 152 in 360d30a

if isassigned(S.nzval, k)

, should be sufficient to change k to Int(k) on that line instead of adding isassigned methods.

KristofferC · 2017-02-11T20:58:58Z

Cool, thanks! I'll update it and run through CI. What do you think about a PR for this method for isassigned though? It feels like it should work?

tkelman · 2017-02-11T21:01:10Z

Good question (mostly a separate one from this PR though), I guess it depends on whether you think of the "canonical index" to be Int for asking if something is assigned, or if isassigned is no more special than getindex or other functions where you want them to be able to take other Integer types.

tkelman · 2017-02-11T21:04:31Z

base/sparse/sparsevector.jl

    limit::Bool = get(io, :limit, false)
    half_screen_rows = limit ? div(displaysize(io)[1] - 8, 2) : typemax(Int)
    pad = ndigits(n)
-    sep = "\n\t"
    if !haskey(io, :compact)
        io = IOContext(io, :compact => true)
    end
    for k = 1:length(nzind)
        if k < half_screen_rows || k > xnnz - half_screen_rows
            print(io, "  ", '[', rpad(nzind[k], pad), "]  =  ")
            Base.show(io, nzval[k])


guess this should probably also be

if isassigned(nzval, Int(k)) Base.show(io, nzval[k]) else print(io, Base.undef_ref_str) end

there just aren't tests of putting a possibly-undef type in as the values of a SparseVector and showing it?

Hmm, sparse vector and matrix doesnt do the same here:

julia> A = speye(3) 3×3 SparseMatrixCSC{Float64,Int64} with 3 stored entries: [1, 1] = 1.0 [2, 2] = 1.0 [3, 3] = 1.0 julia> similar(A, T12960) 3×3 SparseMatrixCSC{T12960,Int64} with 3 stored entries: [1, 1] = #undef [2, 2] = #undef [3, 3] = #undef julia> a = sparsevec([1, 2, 3]) 3-element SparseVector{Int64,Int64} with 3 stored entries: [1] = 1 [2] = 2 [3] = 3 julia> similar(a, T12960) 3-element SparseVector{T12960,Int64} with 0 stored entries

Something else I found:

julia> a = speye(3,3) 3×3 SparseMatrixCSC{Float64,Int64} with 3 stored entries: [1, 1] = 1.0 [2, 2] = 1.0 [3, 3] = 1.0 julia> b = similar(a, Float64, Int64) 3×3 SparseMatrixCSC{Float64,Int64} with 3 stored entries: [1, 1] = 6.92397e-310 [2, 2] = 6.92397e-310 [3, 3] = 6.92397e-310 julia> Base.SparseArrays.dropstored!(a, 1, 1) 3×3 SparseMatrixCSC{Float64,Int64} with 2 stored entries: [2, 2] = 1.0 [3, 3] = 1.0 julia> b 3×3 SparseMatrixCSC{Float64,Int64} with 2 stored entries: [2, 2] = 6.92397e-310 [3, 3] = 6.92397e-310

a and b alias their internal representation. Seems odd.

I feel like I've complained about that partial aliasing somewhere before. Sacha might remember.

I feel like I've complained about that partial aliasing somewhere before. Sacha might remember.

Yes, and with convert :). The dormant #17507 targets that aliasing issue's incarnation for similar. Best!

KristofferC · 2017-02-12T15:46:01Z

pushed some updates with regards to the previous discussion

tkelman · 2017-02-12T19:38:45Z

base/sparse/sparsevector.jl

@@ -758,7 +760,11 @@ function show(io::IOContext, x::AbstractSparseVector)
    for k = 1:length(nzind)
        if k < half_screen_rows || k > xnnz - half_screen_rows
            print(io, "  ", '[', rpad(nzind[k], pad), "]  =  ")
-            Base.show(io, nzval[k])
+            if isassigned(nzval, Int(k))
+                 show(io, nzval[k])


indent is off (sorry if you copy pasted that from my example?)

tkelman · 2017-02-12T19:39:44Z

base/sparse/sparsevector.jl

@@ -35,6 +35,8 @@ count(x::SparseVector) = count(x.nzval)
 nonzeros(x::SparseVector) = x.nzval
 nonzeroinds(x::SparseVector) = x.nzind

+similar(x::SparseVector, Tv::Type=eltype(x)) = SparseVector(x.n, copy(x.nzind), Array{Tv}(length(x.nzval)))
+similar{Tv,Ti}(x::SparseVector, ::Type{Tv}, ::Type{Ti}) = SparseVector(x.n, copy!(similar(x.nzind, Ti), x.nzind), copy!(similar(x.nzval, Tv), x.nzval))


getting long for a single line

tkelman · 2017-02-12T19:41:38Z

test/sparse/sparse.jl

@@ -1729,3 +1729,12 @@ end
    show(io, MIME"text/plain"(), spzeros(Float32, Int64, 2, 2))
    @test String(take!(io)) == "2×2 SparseMatrixCSC{Float32,Int64} with 0 stored entries"
 end
+
+@testset "similar aliasing" begin
+    a = sparse(rand(3,3))


rand can technically return exactly 0 in very rare cases, can't it?

Not sure, I tried to make it happen but it never did.

KristofferC · 2017-02-12T19:53:00Z

Updated

tkelman · 2017-02-12T20:01:11Z

test/sparse/sparsevector.jl

+type t20488 end
+
+@testset "similar" begin
+    x = sparsevec(rand(3))


here as well

done on both

tkelman · 2017-02-12T20:01:55Z

test/sparse/sparsevector.jl

+    @test String(take!(io)) == "1-element SparseVector{Float64,Int64} with 1 stored entry:\n  [1]  =  1.0"
+    show(io, MIME"text/plain"(),  spzeros(Float64, Int64, 2))
+    @test String(take!(io)) == "2-element SparseVector{Float64,Int64} with 0 stored entries"
+    show(io, similar(sparsevec(rand(3)), t20488))


KristofferC added the domain:arrays:sparse Sparse arrays label Feb 7, 2017

KristofferC force-pushed the kc/the_doctest_club branch from 3becc15 to 94fdbab Compare February 7, 2017 09:19

KristofferC force-pushed the kc/the_doctest_club branch from 94fdbab to 5526b5f Compare February 7, 2017 12:36

KristofferC changed the title ~~fixup sparse vector printing and add some doctests to sparse~~ make sparse arrays print more consistently, also add some doctests Feb 7, 2017

KristofferC added the domain:display and printing Aesthetics and correctness of printed representations of objects. label Feb 7, 2017

fredrikekre reviewed Feb 7, 2017

View reviewed changes

StefanKarpinski approved these changes Feb 7, 2017

View reviewed changes

KristofferC force-pushed the kc/the_doctest_club branch from 97258a3 to 03d904a Compare February 7, 2017 15:55

KristofferC force-pushed the kc/the_doctest_club branch from 03d904a to d90276a Compare February 7, 2017 20:09

Sacha0 reviewed Feb 7, 2017

View reviewed changes

Sacha0 approved these changes Feb 7, 2017

View reviewed changes

kshyatt added the domain:docs This change adds or pertains to documentation label Feb 8, 2017

tkelman reviewed Feb 8, 2017

View reviewed changes

Sacha0 mentioned this pull request Feb 8, 2017

dropstored! had one job #20513

Closed

KristofferC added 2 commits February 9, 2017 09:12

make sparse and dense arrays print more similarly

38b421c

update doctest after dropstored fix

14d79c2

KristofferC force-pushed the kc/the_doctest_club branch from d90276a to 14d79c2 Compare February 9, 2017 08:22

KristofferC added 2 commits February 9, 2017 20:24

add extra method for isassigned for mixed indices

e87c7f2

change confusing doctest

58b0401

KristofferC added 2 commits February 11, 2017 18:54

Merge branch 'master' into kc/the_doctest_club

b97ef32

fixup

360d30a

change adding isassigned to Int on call ot it

0a0b541

tkelman reviewed Feb 11, 2017

View reviewed changes

various fixes

97e788e

tkelman reviewed Feb 12, 2017

View reviewed changes

address review

0304012

tkelman reviewed Feb 12, 2017

View reviewed changes

more .+1

4c79d7b

KristofferC merged commit cee29b0 into master Feb 13, 2017

KristofferC deleted the kc/the_doctest_club branch February 13, 2017 07:50

fredrikekre mentioned this pull request May 24, 2017

(Minor) Rephrase 'nonzero entries' in display of SparseMatrix #19119

Closed

make sparse arrays print more consistently, also add some doctests #20488

make sparse arrays print more consistently, also add some doctests #20488

Conversation

KristofferC commented Feb 7, 2017 • edited Loading

KristofferC commented Feb 7, 2017

fredrikekre commented Feb 7, 2017

KristofferC commented Feb 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC commented Feb 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sacha0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC commented Feb 9, 2017

KristofferC commented Feb 10, 2017

tkelman commented Feb 11, 2017

KristofferC commented Feb 11, 2017 • edited Loading

tkelman commented Feb 11, 2017

KristofferC commented Feb 11, 2017

tkelman commented Feb 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC Feb 11, 2017 • edited Loading

Choose a reason for hiding this comment

KristofferC Feb 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sacha0 Feb 11, 2017 • edited Loading

Choose a reason for hiding this comment

KristofferC commented Feb 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC commented Feb 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC commented Feb 7, 2017 •

edited

Loading

KristofferC commented Feb 11, 2017 •

edited

Loading

KristofferC Feb 11, 2017 •

edited

Loading

KristofferC Feb 11, 2017 •

edited

Loading

Sacha0 Feb 11, 2017 •

edited

Loading

KristofferC commented Feb 12, 2017 •

edited

Loading