Lower _linalg_svd #4537
Merged
Lower _linalg_svd #4537
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wonjoolee95
force-pushed
the
lower-linalg-svd
branch
from
e75c513 to
255f999
Compare
|
CI failing due to seemingly unrelated error: |
wonjoolee95
force-pushed
the
lower-linalg-svd
branch
from
255f999 to
f6651cb
Compare
|
Update: this test seems like it's failing on head. |
JackCaoG
approved these changes
Jan 31, 2023
This change lowers _linalg_svd by mostly reusing the existing svd lowering. However, there are a couple of notable differences between these two ops: 1. torch.svd()'s `some` is the opposite of torch.linalg.svd()'s `full_matrices`. Note that default value for both is True, so the default behavior is effectively the opposite. 2. torch.svd() returns V, whereas torch.linalg.svd() returns Vh (V transposed). 3. If `compute_uv` is False, torch.svd() returns zero-filled tensors for U and Vh, whereas torch.linalg.svd() returns empty tensors.
wonjoolee95
force-pushed
the
lower-linalg-svd
branch
from
f6651cb to
080f7c1
Compare
|
crashed with frame info seems like some shape in SVD is invalid.. |
|
Seems like I missed an op in the |
wonjoolee95
force-pushed
the
lower-linalg-svd
branch
from
b391a34 to
3a4deb8
Compare
|
Added some more ops failing due to the same existing error with Now locally, the tests are passing but I'll let the CI verify. |
JackCaoG
reviewed
Feb 1, 2023
JackCaoG
approved these changes
Feb 1, 2023
mateuszlewko
pushed a commit
that referenced
this pull request
Mar 15, 2023
* Lower _linalg_svd This change lowers _linalg_svd by mostly reusing the existing svd lowering. However, there are a couple of notable differences between these two ops: 1. torch.svd()'s `some` is the opposite of torch.linalg.svd()'s `full_matrices`. Note that default value for both is True, so the default behavior is effectively the opposite. 2. torch.svd() returns V, whereas torch.linalg.svd() returns Vh (V transposed). 3. If `compute_uv` is False, torch.svd() returns zero-filled tensors for U and Vh, whereas torch.linalg.svd() returns empty tensors. * Run linter * Disable some more tests * Add comments to the disabled tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4511
Lower _linalg_svd
This change lowers _linalg_svd by mostly reusing the existing svd lowering. However, there are a couple of notable differences between these two ops:
someis the opposite of torch.linalg.svd()'sfull_matrices. Note that default value for both is True, so the default behavior is effectively the opposite.compute_uvis False, torch.svd() returns zero-filled tensors for U and Vh, whereas torch.linalg.svd() returns empty tensors.As a result of this change, we also disable the following ops in xla/test/test_ops.py:
geqrflinalg.detlinalg.inv_exlinalg.matrix_ranklinalg.svdlinalg.svdvalsThese ops all call into the newly lowered
_linalg_svdand cause failure:Slice dim size 1 greater than dynamic slice dimension: 0. This failure seems to be also happening in our master branch for the already lowered opsvd. In xla/test/test_ops.py, ops such assvd,pinverse, ornuclear_normthat callsvdare failing due to the same error and are disabled. However, to ensure correctness for the newly lowered_linalg_svd, this PR adds unit tests and does not cause any regressing cpp test failures.