JuliaLang / LinearAlgebra.jl

Julia Linear Algebra standard library
Other
17 stars 4 forks source link

Base.show fails with structural matrices with non-number elements #1013

Open longemen3000 opened 1 year ago

longemen3000 commented 1 year ago

Some matrices (SparseMatrixCSC and Diagonal, as far as i know) depend on a type defining iszero(x::T) and zero(Type{T}) to work. a MWE:

show(IOBuffer(),MIME"text/plain"(),Diagonal(fill(nothing,3)))

version info.

julia> versioninfo()
Julia Version 1.9.1
Commit 147bdf428c (2023-06-07 08:27 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: 8 × Intel(R) Core(TM) i7-4720HQ CPU @ 2.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, haswell)
  Threads: 4 on 8 virtual cores
Environment:
  JULIA_NUM_THREADS = 4

related:

https://github.com/korsbo/Latexify.jl/issues/269

longemen3000 commented 1 year ago

this could be solved by:

  1. defining Base.isstored(A::Diagonal,i::Int,j::Int) == i == j ? isstored(A.diag,i) : false

  2. Replacing the code in arrayshow.jl: https://github.com/JuliaLang/julia/blob/36e188f9dcd99555af74dadcfcfdda6a41e348b4/base/arrayshow.jl#L67-L72

to:

if isstored(X,i,j) && isassigned(X,i,j)
    aij = alignment(io, X[i,j])::Tuple{Int,Int}
else
    aij = undef_ref_alignment
end
vtjnash commented 1 year ago

I am not certain about the proposed fix there, since the code expects getindex to work, and then calls Base.replace_in_print_matrix to call Base.replace_with_centered_mark on the resulting element repr.

longemen3000 commented 1 year ago

The issue is not fixed with JuliaLang/julia#50391, but defines the necessary functions to work on a fix

vtjnash commented 1 year ago

Right, github got confused by the text there, but my uncertainty remains over the proposed answer here

longemen3000 commented 1 year ago

Yeah, I will need to see what to do about that, but at least there is now a way to detect if an element of a matrix is stored or not

vtjnash commented 1 year ago

Yeah, that seemed to be the goal of a5e032e29686fabc8b88a10726c79feb18900055 (#33821), which originally started to add that.