Skip to content

Conversation

@vishwakftw
Copy link
Contributor

@vishwakftw vishwakftw commented Jun 29, 2018

  1. Used SVD to compute.
  2. Tests in test_autograd, test_cuda and test_torch
  3. Doc strings in _torch_docs.py and _tensor_docs.py

Closes #6187

1. Used SVD to compute.
2. Tests in test_cuda and test_torch
3. Doc strings in _torch_docs.py and _tensor_docs.py

Closes pytorch#6187
@vishwakftw vishwakftw changed the title Implement torch.pinv : Pseudo-inverse Implement torch.pinverse : Pseudo-inverse Jun 30, 2018
}

Tensor pinverse(const Tensor& self) {
if (!at::isFloatingType(self.type().scalarType()) ||

This comment was marked as off-topic.

}
Tensor U, S, V;
std::tie(U, S, V) = self.svd();
Tensor S_pseudoinv = at::where(S != 0.0, S.reciprocal(), at::zeros({}, self.type()));

This comment was marked as off-topic.

1. Use AT_CHECK
2. Use at::zeros({}, self.options);
@vishwakftw
Copy link
Contributor Author

Not sure why the CUDA tests are failing. @ssnl

"of floating types");
Tensor U, S, V;
std::tie(U, S, V) = self.svd();
Tensor S_pseudoinv = at::where(S != 0.0, S.reciprocal(), at::zeros({}, self.options()));

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@vishwakftw
Copy link
Contributor Author

A few comments: @ssnl

  1. Naming - pinverse instead of pinv: PyTorch has inverse for matrix inverse, hence the convention.
  2. Tests in test_autograd.py: I had a thought about it initially, but pseudoinverses are discontinuous. Derivatives exist for a constant rank only. Ref: https://mathoverflow.net/questions/25778/analytical-formula-for-numerical-derivative-of-the-matrix-pseudo-inverse

@ssnl
Copy link
Collaborator

ssnl commented Jun 30, 2018

Very valid reasons. Thanks for your explanation!

  1. Our current general goal is to align with numpy interface. Unfortunately we didn't have this guideline before. I believe that we probably want to alias inverse with inv in future. @soumith , should we just name this as pinv? Or have both pinv and pinverse as its aliases?

  2. Thanks for the reference. I agree. However, the current implementation is still backprop-able, so users may attempt to backprop. How about this: let's test the following function:

    assumption: 
      1. m <= n
      2. two fixed orthonormal matrices U and V
    input: x of shape [m]
    output: S = torch.cat([x, torch.zeros(n - m)], 0).diag(); return U @ S @ V
    

    If we test it on sufficiently large positive input x, it should guarantee constant rank m.
    Also, given its instability, I suggest we add a .. note:: in its doc (just as we did for svd). I think the note probably should also mention double backward instability from svd (similar to the note in det).

Let me know what you think!

@ssnl
Copy link
Collaborator

ssnl commented Jun 30, 2018

I think, svd has similar issue with U and V matrices when the input is not full rank, but we tested it in the "good" case anyways. :D

@vishwakftw
Copy link
Contributor Author

@ssnl Sounds good to me. I have added those tests in test_autograd.

However, the build is still failing. Could it be a precision problem with GPUs?

@ssnl
Copy link
Collaborator

ssnl commented Jun 30, 2018

It could be. Let's wait until CI finishes.

s = torch.arange(1., l + 1).mul_(1.0 / (l + 1))
return u.mm(torch.diag(s)).mm(v.t())

def random_matrix_large_singular_value(m, n):

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@ssnl
Copy link
Collaborator

ssnl commented Jun 30, 2018

how big is the difference in CUDA failure?

@vishwakftw
Copy link
Contributor Author

About 5.5 - 6 .

@ssnl ssnl mentioned this pull request Jul 1, 2018
facebook-github-bot pushed a commit that referenced this pull request Jul 2, 2018
Summary:
Fixes #9079

There is room for speed-up for both functions (see #9083), but let's get this in to unblock #9052 .
Closes #9082

Reviewed By: ezyang

Differential Revision: D8711687

Pulled By: SsnL

fbshipit-source-id: f043a9bf55cb6aec5126c3331d35761f7aa3f8e3
@vishwakftw
Copy link
Contributor Author

Mac OS build failure seems unrelated.

@vishwakftw
Copy link
Contributor Author

@pytorchbot retest this please

return M.pinverse()

gradcheck(func, [torch.rand(m) + 1])
gradcheck(func, [torch.rand(m) + 10])

This comment was marked as off-topic.

This comment was marked as off-topic.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssnl has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssnl has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ssnl
Copy link
Collaborator

ssnl commented Jul 2, 2018

@pytorchbot retest this please

1 similar comment
@ssnl
Copy link
Collaborator

ssnl commented Jul 2, 2018

@pytorchbot retest this please

@vishwakftw
Copy link
Contributor Author

@pytorchbot retest this please

run_test((10,), 0)

def test_pinverse(self):
m, n = 5, 10

This comment was marked as off-topic.

This comment was marked as off-topic.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssnl has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssnl is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssnl has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ssnl
Copy link
Collaborator

ssnl commented Jul 3, 2018

@vishwakftw The PR is great! Thanks for doing this :).

There are just some internal problems that I'm trying to fix before I can merge this.

@vishwakftw vishwakftw deleted the pinverse branch July 5, 2018 16:16
zdevito pushed a commit to zdevito/ATen that referenced this pull request Jul 5, 2018
Summary:
1. Used SVD to compute.
2. Tests in test_autograd, test_cuda and test_torch
3. Doc strings in _torch_docs.py and _tensor_docs.py

Closes #6187
Closes pytorch/pytorch#9052

Reviewed By: soumith

Differential Revision: D8714628

Pulled By: SsnL

fbshipit-source-id: 7e006c9d138b9f49e703bd0ffdabe6253be78dd9
zdevito pushed a commit to zdevito/ATen that referenced this pull request Jul 13, 2018
Summary:
1. Used SVD to compute.
2. Tests in test_autograd, test_cuda and test_torch
3. Doc strings in _torch_docs.py and _tensor_docs.py

Closes #6187
Closes pytorch/pytorch#9052

Reviewed By: soumith

Differential Revision: D8714628

Pulled By: SsnL

fbshipit-source-id: 7e006c9d138b9f49e703bd0ffdabe6253be78dd9
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
Summary:
Fixes pytorch#9079

There is room for speed-up for both functions (see pytorch#9083), but let's get this in to unblock pytorch#9052 .
Closes pytorch#9082

Reviewed By: ezyang

Differential Revision: D8711687

Pulled By: SsnL

fbshipit-source-id: f043a9bf55cb6aec5126c3331d35761f7aa3f8e3
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
Summary:
1. Used SVD to compute.
2. Tests in test_autograd, test_cuda and test_torch
3. Doc strings in _torch_docs.py and _tensor_docs.py

Closes pytorch#6187
Closes pytorch#9052

Reviewed By: soumith

Differential Revision: D8714628

Pulled By: SsnL

fbshipit-source-id: 7e006c9d138b9f49e703bd0ffdabe6253be78dd9
@mscipio
Copy link

mscipio commented Dec 18, 2018

It could be useful to adapt torch.pinverse to work with batches of tensors like done for torch.inverse in v1.0.0

@vishwakftw
Copy link
Contributor Author

Hi @mscipio, batching for pinverse is blocked due to unavailability of batched SVD, which is used internally to compute the pinverse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pseudoinverse

5 participants