compute_uv for SVD #12517
compute_uv for SVD #12517
Conversation
vishwakftw
requested review from
apaszke,
colesbury,
ezyang,
gchanan,
soumith and
zdevito
as code owners
- I didn't change the implementation for the nuclear_norm function since it was a differentiable op earlier. Implementation for matrix_rank has been changed though
vishwakftw
force-pushed
the
svd-compute_uv
branch
from
cae7482 to
26b5fc9
Compare
| Tensor svd_backward(const std::vector<torch::autograd::Variable> &grads, const Tensor& self, | ||
| bool some, const Tensor& raw_u, const Tensor& sigma, const Tensor& raw_v) { | ||
| bool some, bool compute_uv, const Tensor& raw_u, const Tensor& sigma, const Tensor& raw_v) { | ||
| if (!compute_uv) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
I wish there is an easy way to dispatch to different native_functions basing on GradMode and inputs' requires_grad flags.... |
|
@ssnl Is this good to go? |
| bool some, const Tensor& raw_u, const Tensor& sigma, const Tensor& raw_v) { | ||
| bool some, bool compute_uv, const Tensor& raw_u, const Tensor& sigma, const Tensor& raw_v) { | ||
| AT_CHECK(compute_uv, | ||
| "cannot compute backward for SVD without computing the singular matrices in the forward pass"); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
I also wonder how much faster this is. Could you do some benchmarking on CPU (MKL) & GPU (MAGMA)? Thanks! |
|
CPU benchmarks:
GPU benchmarks can be expected to have the same speedup, since both use the divide and conquer algorithm ( |
| bool some, const Tensor& raw_u, const Tensor& sigma, const Tensor& raw_v) { | ||
| bool some, bool compute_uv, const Tensor& raw_u, const Tensor& sigma, const Tensor& raw_v) { | ||
| AT_CHECK(compute_uv, | ||
| "Setting compute_uv to false doesn't compute singular matrices, and hence we cannot compute backward ", |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@ssnl Is this good to go now? |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
SsnL is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Adds a `compute_uv` argument that defaults to `True` for optionally computing the singular vectors during SVD. Closes pytorch/pytorch#12420 . Pull Request resolved: pytorch/pytorch#12517 Differential Revision: D10384554 Pulled By: SsnL fbshipit-source-id: 704998a257afa815eda901b8ae830e8a661695be
5 participants
Adds a
compute_uvargument that defaults toTruefor optionally computing the singular vectors during SVD.Closes #12420 .