LinearAlgebra.jl icon indicating copy to clipboard operation
LinearAlgebra.jl copied to clipboard

QR factorization does not show Q factor

Open cafaxo opened this issue 2 years ago • 9 comments

On current master, I get

julia> qr(randn(2,2))
LinearAlgebra.QRCompactWY{Float64, Matrix{Float64}, Matrix{Float64}}
Q factor:
2×2 LinearAlgebra.QRCompactWYQ{Float64, Matrix{Float64}, Matrix{Float64}}
R factor:
2×2 Matrix{Float64}:
 -2.3358  -0.951623
  0.0     -0.608775

The actual matrix should be printed after Q factor:.

cafaxo avatar May 04 '23 17:05 cafaxo

This was also changed in JuliaLang/julia#46196 on purpose. Qs are not readily accessible in memory, their components need to be computed one by one, so printing them like a matrix is costly. If you're interested in seeing the matrix representation of something that is not in matrix format, you should call Matrix(qr(...).Q).

dkarrasch avatar May 05 '23 15:05 dkarrasch

Can this be closed?

dkarrasch avatar May 09 '23 10:05 dkarrasch

I think we shouldn't care too much about cost when printing / showing a struct.

cafaxo avatar May 09 '23 11:05 cafaxo

I've looked into it. It's a pretty big deal, after all. We have some matrix printing code that does all computations regarding alignment of elements, fill-up with ellipses etc. That's specialized to AbstractVecOrMat, which AbstractQ isn't. Removing the type constraints in Julia Base doesn't seem like an option, the other options are (i) first compute the entire matrix and display it as usual (that is certainly expensive and may make the repl look like it stalled, I think we were discussing this over at SparseArrays.jl with @jishnub IIRC), or (ii) duplicate the printing code, which is also bad. All the effort for printing something like a matrix that isn't a matrix. 🤔

dkarrasch avatar May 10 '23 10:05 dkarrasch

Is there a different way to show it, perhaps by summarising the struct fields? The current display might confuse users who might expect some value to be printed after the type

jishnub avatar May 10 '23 10:05 jishnub

Why not just print Matrix(Q)? Was the matrix previously allocated when printing Q?

cafaxo avatar May 10 '23 12:05 cafaxo

Given that AbstractQ used to be an AbstractMatrix, it could avail the show methods for AbstractArrays that only needed to compute a handful of elements. Now that it's not an AbstractMatrix anymore, it lacks an efficient show method. Computing the entire matrix representation will be expensive and may allocate considerably, which is unnecessary for an object that isn't really a Matrix.

jishnub avatar May 10 '23 13:05 jishnub

Is that more expensive than before?

Of course. You'd allocate an entirely new matrix in memory.

Was the matrix previously allocated when printing Q?

No, for big Q's, you'd only want to print the "corners" of the "matrix". In that case, you better compute only those that are required for printing, instead of first computing everything and then use only the corners. That's what used to be done. Printing works via getindex, and for Q's that means computing columns or elements etc.

dkarrasch avatar May 10 '23 13:05 dkarrasch

What if we change the output to

julia> qr(randn(2,2))
LinearAlgebra.QRCompactWY{Float64, Matrix{Float64}, Matrix{Float64}}
Q factor: 2×2 LinearAlgebra.QRCompactWYQ{Float64, Matrix{Float64}, Matrix{Float64}}
R factor:
2×2 Matrix{Float64}:
 -2.3358  -0.951623
  0.0     -0.608775

? Like remove the line break between "Q factor" and the summary of Q. If one really wants to inspect Q, one will be always better off computing the entire matrix, or columns of interest. Anything is better than elementwise.

dkarrasch avatar May 10 '23 15:05 dkarrasch

I think this can safely be closed. @dkarrasch's last comment has long since been implemented, and computing Q explicitly for printing is not going to happen.

araujoms avatar May 12 '25 08:05 araujoms

Yes, for reference, this was at least improved in https://github.com/JuliaLang/julia/pull/49714.

dkarrasch avatar May 12 '25 09:05 dkarrasch