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

Implement complex QR decomposition in HLO (TPU) #1274

Closed
mganahl opened this issue Aug 30, 2019 · 8 comments
Closed

Implement complex QR decomposition in HLO (TPU) #1274

mganahl opened this issue Aug 30, 2019 · 8 comments
Labels
enhancement New feature or request

Comments

@mganahl
Copy link

mganahl commented Aug 30, 2019

The following code raises RuntimeError: Unimplemented: complex comparison 'LT'

import jax
jax.np.linalg.qr((np.random.rand(3, 3) + 1j * np.random.rand(3, 3)))

Noticed this because one of our test cases fails (google/TensorNetwork#221)

@mattjj mattjj added the bug Something isn't working label Sep 2, 2019
@hawkinsp
Copy link
Collaborator

hawkinsp commented Sep 3, 2019

The QR implementation we are using is implemented in XLA/HLO on all platforms and only supports real inputs.

On CPU and GPU we should really be calling LAPACK and Cusolver implementations of QR decomposition anyway. Both libraries support complex inputs and would probably perform better than the HLO implementation on their respective platforms.

On TPU we would need to add complex number support here:
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/xla/client/lib/qr.cc

Which platform are you interested in?

@hawkinsp hawkinsp added enhancement New feature or request and removed bug Something isn't working labels Sep 3, 2019
@hawkinsp hawkinsp self-assigned this Sep 3, 2019
@mganahl
Copy link
Author

mganahl commented Sep 3, 2019

Hi! We'd be interested in all three. The ranking is CPU, GPU and TPU, with TPU the least important right now. Thanks for the quick reply!

@hawkinsp
Copy link
Collaborator

hawkinsp commented Sep 4, 2019

PR #1306 adds complex QR support on CPU and GPU.

TPU is not yet implemented, so I'll leave this issue open and retitle it.

@hawkinsp hawkinsp changed the title complex qr crashes Implement complex QR decomposition in HLO (TPU) Sep 4, 2019
@hawkinsp hawkinsp removed their assignment Sep 4, 2019
@mganahl
Copy link
Author

mganahl commented Sep 4, 2019

awesome thanks!

@long1216
Copy link

long1216 commented Aug 6, 2020

I got a similar error, from #3862, RuntimeError: Unimplemented: complex comparison 'GE'
Does they belong to the same problem, also related to the complex comparison in XLA? What the meaning of the terms 'GE', 'LT' stand for? Thank you so much.

@mattjj
Copy link
Collaborator

mattjj commented Aug 6, 2020

GE and LT are "greater than" and "less than", respectively. That error is saying that those operations aren't implemented for complex numbers. (NumPy implements them with a kind of weird convention, since complex numbers don't have a total ordering.)

@long1216
Copy link

long1216 commented Aug 6, 2020

Thank you for your answer, Matthew. Would there be any plans for finalizing the conversion of complex comparison? Could we just follow the convention of NumPy in XLA first? Thank you.

@hawkinsp
Copy link
Collaborator

hawkinsp commented Dec 2, 2020

We've already fixed this at head!

@hawkinsp hawkinsp closed this as completed Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants