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

Add a tridiagonal eigh solver. #6622

Merged
merged 1 commit into from May 5, 2021
Merged

Conversation

hawkinsp
Copy link
Member

@hawkinsp hawkinsp commented May 3, 2021

This code was written by Rasmus for TensorFlow; this PR ports it to JAX.

@google-cla google-cla bot added the cla: yes label May 3, 2021
@hawkinsp hawkinsp added the pull ready Ready for copybara import and testing label May 3, 2021
@hawkinsp hawkinsp force-pushed the eightr branch 2 times, most recently from 112ecc6 to 41b2bff Compare May 3, 2021 15:57
beta_abs = jnp.abs(beta)
beta_sq = beta * beta

# Compute an interval containing the eigenvalue of T using the Gerschgorin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Compute an interval containing the eigenvalue of T using the Gerschgorin
# Compute an interval containing the eigenvalue of T using the Gershgorin

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://en.wikipedia.org/wiki/Semyon_Aranovich_Gershgorin says this is an accepted transliteration!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay you win, though the spelling is inconsistent with a comment a few lines down. (Maybe that's good, as all possible transliterations are represented.)

Copy link
Member

@mattjj mattjj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Since GitHub already marked me as a reviewer, given my drive-by comment...)

Beautiful code as always!

@hawkinsp
Copy link
Member Author

hawkinsp commented May 5, 2021

(Since GitHub already marked me as a reviewer, given my drive-by comment...)

Beautiful code as always!

To be clear, Rasmus gets all the credit for this code. I just ported it, which happily at the level of NumPy-like ops is almost completely mechanical to do.

(The autodiff stuff, when we have the pieces for it, will be a bit different.)

@mattjj
Copy link
Member

mattjj commented May 5, 2021

Copying Rasmus code is a recipe for success in any endeavor! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants