Lower _linalg_svd #4537
Merged
Lower _linalg_svd #4537
Conversation
This file contains hidden or 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
wonjoo-wj force-pushed the lower-linalg-svd branch from e75c513 to 255f999 Compare | CI failing due to seemingly unrelated error: |
wonjoo-wj 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.
wonjoo-wj 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 |
wonjoo-wj 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
2 participants
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.