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

QR factorization does not show Q factor #49635

Open
cafaxo opened this issue May 4, 2023 · 9 comments
Open

QR factorization does not show Q factor #49635

cafaxo opened this issue May 4, 2023 · 9 comments
Labels
display and printing Aesthetics and correctness of printed representations of objects. linear algebra Linear algebra

Comments

@cafaxo
Copy link
Contributor

cafaxo commented May 4, 2023

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:.

@dkarrasch
Copy link
Member

dkarrasch commented May 5, 2023

This was also changed in #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 dkarrasch added the display and printing Aesthetics and correctness of printed representations of objects. label May 5, 2023
@dkarrasch
Copy link
Member

Can this be closed?

@cafaxo
Copy link
Contributor Author

cafaxo commented May 9, 2023

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

@dkarrasch
Copy link
Member

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. 🤔

@jishnub
Copy link
Contributor

jishnub commented May 10, 2023

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

@cafaxo
Copy link
Contributor Author

cafaxo commented May 10, 2023

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

@jishnub
Copy link
Contributor

jishnub commented May 10, 2023

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.

@dkarrasch
Copy link
Member

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
Copy link
Member

dkarrasch commented May 10, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects. linear algebra Linear algebra
Projects
None yet
Development

No branches or pull requests

3 participants