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

Computing generalized eigenvalues #127

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

daanhb
Copy link

@daanhb daanhb commented Jun 3, 2023

This is the PR mentioned in #126 and based on code provided to me by @thijssteel.

It implements eigval(A, B) for generic matrices A and B using the QZ algorithm. Copying a comment from the code:

Two orthogonal matrices Q and Z are constructed to reduce the "pencil" (A,B) to another form Q'*(A,B)*Z:

  • in a first step A and B are reduced to upper Hessenberg and upper triangular form, respectively (see: hesstriangular)
  • next a generalized Schur decomposition further reduces both matrices to upper triangular form
  • the generalized eigenvalues are then given by the ratios of the diagonals of those two triangular matrices.

@daanhb
Copy link
Author

daanhb commented Jun 3, 2023

I have yet to add tests. First I'm wondering whether this package is the right place to add this functionality and whether it's deemed useful. It is at least to me :-)

@codecov
Copy link

codecov bot commented Jun 3, 2023

Codecov Report

Patch coverage has no change and project coverage change: -23.35 ⚠️

Comparison is base (27a78e0) 96.08% compared to head (6a5beeb) 72.74%.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #127       +/-   ##
===========================================
- Coverage   96.08%   72.74%   -23.35%     
===========================================
  Files          11       12        +1     
  Lines        1636     2161      +525     
===========================================
  Hits         1572     1572               
- Misses         64      589      +525     
Impacted Files Coverage Δ
src/GenericLinearAlgebra.jl 100.00% <ø> (ø)
src/eigen.jl 97.17% <ø> (ø)
src/qz.jl 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@daanhb
Copy link
Author

daanhb commented Jun 3, 2023

Looking at this code, at other parts of GenericLinearAlgebra and at LinearAlgebra, I see many opportunities for better integration (and less code in total). For this PR:

  • reuse LinearAlgebra.floatmin2
  • better share code for Givens rotations, Householder reflections and eigenvalues of 2x2 blocks
  • make computation of eigenvalues more consistent. LinearAlgebra returns real eigenvalues when they are real, GenericLinearAlgebra always returns complex vectors (it seems?) and I've followed LinearAlgebra

It might make the implementation quite a bit shorter.

@andreasnoack
Copy link
Member

I'm sorry for the silence here. This contributions is highly appreciated. I'm a bit stretched for the time being but will try to get this reviewed over the summer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants