-
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 svd #28770
base: main
Are you sure you want to change the base?
Fix torch svd #28770
Conversation
… functions, update the docstring of ivy.svd to mention the change of content of return when compute_uv is False, update svd's torch backend to only compute the necessary second component to be more efficient and the relevant docstring in ivy.svd as well
…ple definition and behavior when compute_uv is false
@Sam-Armstrong Can you tell me about how to get the device of the input received by the torch frontend function svd? Because the documentation requires the zero-filled matrixes in the output should be on the same device as input (https://pytorch.org/docs/stable/generated/torch.svd.html). |
@Daniel4078 you should just be able to do |
…ple definition and behavior when compute_uv is false
@Sam-Armstrong I found something strange when I run the test locally. The test's error is like this
Here the return from both the ground truth function and ourfrontend are named tuples (U,S,V) where U and V should be zero matrixs (just like what is in the "values" variable). It seems like the assertion check somehow pick the first two part of one returned tuple and consider them as the frontend-groundtruth pair. I think maybe we should investigate this further as |
@Daniel4078 isn't the problem here that the shape of the tensors in the named tuple is different to the ground truth? Like the first element U of the tuple is 3d in the ground truth but 2d in the function return |
@Sam-Armstrong I see, comparing to the document of ivy.svd, it seems like the document of torch.svd assume the input is 2D and is incomlete. |
…fixed the wrong shape and dtype of return. Now there are somehow numerial difference between return of groundtruth torch.svd and ivy.svd
@Sam-Armstrong the tests now sometime fails due to the fact that the correct output of svd is not unique and I am thinking of copying the test for ivy.linalg.svd(
|
@Daniel4078 sure, think that makes sense. yeah that would be great if you can fix the other failing frontend svd tests in this PR, thanks! |
ef20966
to
dce10a6
Compare
It is quite surprising that now torch.blas_and_lapack_ops.svd, torch.tensor.svd, jax.lax.linalg.svd, jax.numpy.linalg.svd, numpy.linalg.decompositions.svd are failing; while torch.linalg.svd , tensorflow.linalg.svd and tensorflow.raw_ops.svd are skipped. The only passing svd test is that of ivy functional API core. |
@Daniel4078 you haven't made any changes to the tests here, did you force-push over them? |
I accidently commited some other changes intended in another branch in here, so I thought it would be better to remove that commit. |
…y of result instead of matching values
@Sam-Armstrong Why all the torch.svd frontend tests are skipping when I run it locally, showing that "Skipped: Function not implemented in backend." What should I do in this case? |
… Can't call numpy() on Tensor that requires grad. Use tensor.detach().numpy() instead. " happen
trying to solve the TypeError: iteration over a 0-d array error when comparing two (somehow empty) result from the matric calculation |
… instead of the normal tuple and apply the np.asarray(x) change over ivy.as_numpy in other tests, ensure the different return pattern (s,u,v tha u,s,v)from tensorflow and when comput_uv is false get handled in the tests. Now only dtype mismatches happens
…timeError: Can't call numpy() on Tensor that requires grad. Use tensor.detach().numpy() instead." when the test function calls np.asarray to the returned value
…ubset_by_index parameter. tensorflow,torch and paddle backends are all failing due to significant value differences in only part of the reconstructed matrixes.
….svd produces significant difference with compute_uv=True (like flipped signs) in the reconstructed matrixes for all backends. I suspect that the decomposition of jax.lax.linalg.svd is different from other similiar functions
it is strange that with the same calculation to reconstruct the decomposited matrix (used in all other relevant tests), current jax.numpy.linalg.svd passes all tests while current jax.lax.linalg.svd fails with significantly different values produced. May be the decomposition of jax.lax.linalg.svd is different from the common way? |
…st_torch_svd, all tests passed
…ecompositions.py::test_numpy_svd, though seems like using ivy.astype to convert type does not work at all
….py::test_tensorflow_Svd, now the frontend somehow always return float64 nomatter input dtype
…lg.py::test_jax_svd
….py::test_tensorflow_Svd, though if input is complex number the results show large difference
…pack_ops.py::test_torch_svd so that complex number inputs are also tested, they are passing
…are also tested, they are all passing
…turn fixed dtypes depending on if input is complex,
…alue instead as the same as the input, but the documents shows that the S component will always be real, which is different from the groundtruth behavior shown in the, updated the jax svd tests to check for complex inputs
….svd fail for tensorflow backend due to significant and updated all relavent svd with more complex supported dtype
found that the svd function of tenserflow backend does not support complex input, this may be the reason why some of the frontend tests for svd returns wrong results in tensorflow backend for complex value inputs. Also, I want to ask is the first component of the return of the helpers.test_frontend_function representing the groundtruth return? |
1 similar comment
found that the svd function of tenserflow backend does not support complex input, this may be the reason why some of the frontend tests for svd returns wrong results in tensorflow backend for complex value inputs. Also, I want to ask is the first component of the return of the helpers.test_frontend_function representing the groundtruth return? |
PR Description
Related Issue
Closes #28769
Checklist
Socials