-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Fix torch linalg cholesky_ex #28773
Fix torch linalg cholesky_ex #28773
Conversation
@Sam-Armstrong This experimental function is supposed to quckly and directly returning the LAPACK error codes (https://www.netlib.org/lapack/lug/node136.html) as its return when the normal cholesky would fail, by directly checking for the error before calcuating the cholesky value (https://pytorch.org/docs/stable/generated/torch.linalg.cholesky_ex.html). Though I don't know how to implement and test for that behavior. Though the tests are passing for now it seems. |
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 can just return a regular tuple rather than a named tuple, I know the torch function returns a named tuple, but practically I don't think there would really be a difference and I'm concerned that the named tuple could possibly cause problems for the transpiler. Otherwise the changes look good. Thanks @Daniel4078!
I don't think you need to implement/test the LAPACK error codes right now, that's probably out of scope for the moment. |
The reason why I use named tuple is that some other backend functions like ivy.svd did already return named tuple ( ivy/ivy/functional/ivy/linear_algebra.py Line 2143 in 5665437
|
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.
Ah yeah, I think this is fine on second thought. Thanks a lot @Daniel4078!
PR Description
Related Issue
Closes #28772
Checklist
Socials