-
Notifications
You must be signed in to change notification settings - Fork 60
Add support for complex64 in QR decomposition #2040
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
base: main
Are you sure you want to change the base?
Add support for complex64 in QR decomposition #2040
Conversation
Updated the QR decomposition function to support complex64 data type in addition to float32 and float64.
Fixed syntax errors from incorrect placement of complex64 in the dtype check. Changed 'if A.dtype not in [float32, float64], complex64:' to correct syntax 'if A.dtype not in [float32, float64, complex64]:'. Removed extraneous ', complex64' text from lines 100-102 that were incorrectly added.
|
Thank you for the PR! |
|
Thank you for the PR! |
|
@2400032275 thanks for taking up this issue 👍. I have two suggestions if you want to complete this PR:
If you need any help, feel free to ask :) |
brownbaerchen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have two ways of proceeding
- Remove any type checking and just leave it to torch to decide what it accepts for serial factorization
- Use the general type check with
issubdtype
If the first one works, I would prefer that, but I am not sure. So maybe stick with the latter.
I definitely agree that this cannot be merged without being tested.
| procs_to_merge = A.comm.size | ||
|
|
||
| if A.dtype not in [float32, float64]: | ||
| if A.dtype not in [float32, float64, complex64]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better check would be
from ..types import issubdtype, floating, complex
if not issubdtype(A.dtype, floating) and not issubdtype(A.dtype, complex):which would automatically work with all precision versions of these types. In particular, ones that will be added in the future.
Fixes #2039
Updated the QR decomposition function to support complex64 data type in addition to float32 and float64.
Due Diligence
Description
Issue/s resolved: #
Changes proposed:
Type of change
Memory requirements
Performance
Does this change modify the behaviour of other functions? If so, which?
yes / no