Added linalg.pinv #48399

IvanYashchuk · 2020-11-23T16:48:36Z

This PR adds torch.linalg.pinv.

Changes compared to the original torch.pinverse:

New kwarg "hermitian": with hermitian=True eigendecomposition is used instead of singular value decomposition.
rcond argument can now be a Tensor of appropriate shape to apply matrix-wise clipping of singular values.
Added out= variant (allocates temporary and makes a copy for now)

Ref. #42666

dr-ci · 2020-11-23T17:08:25Z

💊 CI failures summary and remediations

As of commit 48ac5dd (more details on the Dr. CI page):

1/1 failures possibly* introduced in this PR
- 1/1 non-CircleCI failure(s)

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

This comment has been revised 47 times.

codecov · 2020-12-10T02:28:29Z

Codecov Report

Merging #48399 (48ac5dd) into master (eb87686) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #48399   +/-   ##
=======================================
  Coverage   80.70%   80.71%           
=======================================
  Files        1904     1904           
  Lines      206598   206645   +47     
=======================================
+ Hits       166741   166792   +51     
+ Misses      39857    39853    -4

mruberry · 2020-12-15T06:51:23Z

aten/src/ATen/native/LinearAlgebra.cpp

-              "pinverse(", self.scalar_type(), "{", self.sizes(), "}): expected a tensor with 2 or more dimensions "
+Tensor linalg_pinv(const Tensor& input, const Tensor& rcond, bool hermitian) {
+  TORCH_CHECK((at::isFloatingType(input.scalar_type()) || at::isComplexType(input.scalar_type())) && input.dim() >= 2,
+              "linalg_pinv(", input.scalar_type(), "{", input.sizes(), "}): expected a tensor with 2 or more dimensions "
              "of floating types");


"of floating types" is misleading. The requirement is that the tensors have a floating point dtype or a complex dtype.

Also, is this the right place to valid that input and rcond have the same dtype?

Also also, what happens when the dtype is bfloat16 or half?

Oh wait, I'm mistaken. It should not be that the dtype of input and rcond are the same.

Shouldn't it be that rcond must always be the "value type" of input? That is, if input is double rcond should be double, but if input is complex double then rcond should be double, right?

I changed the torch check to accept only float, double, cfloat or cdouble types.

As for the rcond, I don't know actually how we should restrict its types. When only a float is given from Python interface it always gets converted to double in C++ and then we always create a scalar tensor of type double.
If directly a tensor is passed, then all types should be valid that allow multiplication with max_val tensor (that can be only of type float or double) and that allow comparison with a tensor of type float or double. So I guess any floating and integer types (so everything that is not complex) should be valid here for any allowed input?

I added restriction for rcond type not to accept complex types 4de9786
And added tests to show that it works for all other types 4ac1eaf

Your analysis makes sense. Restricting it to not be complex sounds good.

mruberry · 2020-12-15T06:52:16Z

aten/src/ATen/native/LinearAlgebra.cpp

+    return at::empty(input_sizes, input.options());
+  }
+
+  Tensor rcond_ = rcond;


Can you elaborate on what's happening here, and add a comment elaborating

It was not needed here and I removed it. b7a3eb3

aten/src/ATen/native/LinearAlgebra.cpp

mruberry · 2020-12-15T06:59:16Z

aten/src/ATen/native/LinearAlgebra.cpp

+    // TODO: replace input.svd with linalg_svd
+    std::tie(U, S, V_conj) = input.svd();
+    Tensor max_val = at::narrow(S, /*dim=*/-1, /*start=*/0, /*length=*/1);  // singular values are sorted in descending order
+    Tensor S_pseudoinv = at::where(S > rcond_ * max_val, S.reciprocal(), at::zeros({}, S.options())).to(input.dtype());


Parens around rcond_ * max_val so people don't need to worry about the binding of the comparison operator

Is the .to(input.dtype()) actually necessary here?

Yes, .to(input.dtype()) is necessary because S is real-valued and we need it to convert to complex type to be able to do the matmul. at::matmul, and at::where don't work with mixed real and complex types.
Same discussion: #45819 (comment)

aten/src/ATen/native/LinearAlgebra.cpp

IvanYashchuk · 2020-12-16T19:03:17Z

@mruberry I've updated this pull request, it should be in better shape now. I've replied to most of the comments above.

…type

mruberry · 2020-12-22T18:08:28Z

torch/linalg/__init__.py

+
+Computes the pseudo-inverse (also known as the Moore-Penrose inverse) of a matrix :attr:`input`,
+or of each matrix in a batched :attr:`input`.
+The pseudo-inverse is computed using singular value decomposition (see :func:`torch.linalg.svd`) by default.


Unfortunately torch.linalg.svd doesn't exist yet. This can reference torch.svd for now.

Sure I can change that.
I just thought that we should start using torch.linalg even if the references are not working for now.

While we're not actually using the newer linear algebra functions internally we should still reference them here.
#48206 (comment)

It is nice to dream... but we should probably keep all the references in the docs working ;)

torch/linalg/__init__.py

mruberry · 2020-12-22T18:13:17Z

torch/linalg/__init__.py

+            [-1.1921e-07+0.0000e+00j, -2.3842e-07-2.9802e-07j,
+            1.0000e+00-1.7897e-07j]])
+
+    Non-default rcond example


This example is tricky. Because we don't see the intermediate steps it's hard to understand the effect that rcond is having. The next example has the same issue.

What are your thoughts? I think we can leave out these examples for now since this PR seems to be almost ready to merge, otherwise, and maybe return to add them later?

The documentation says that the pseudo-inverse is calculated using SVD and it says that rcond determines which singular values should be set to zero. The first example demonstrates that using the default and some other value for rcond gives different results as expected. Just using the facts from the docs I think it's understandable that different rconds in general give different results. I think we should keep this example because I think examples should contain the code snippets demonstrating each variable. That's studying math to really understand the effect of rcond (topic of Tikhonov regularization) and definitely not needed here. We're on the level of "something in the input changes, something in the output should change as well".

The purpose of the second rcond example (Matrix-wise rcond example) is to show what shapes of rcond are acceptable. I agree that the second example doesn't look transparent and it might be difficult to understand, but that's broadcasting. Smaller arrays are extended to higher-dimensional arrays. We have rcond must be broadcastable to singular values of input. in the docs. I don't see how to help the user to understand this bit better.

How about keeping these examples as is (that's working and valid code) and return to it later to improve them?
I don't mind removing it now.

@mruberry, so should we remove those rcond examples now and think about it later or keep it?

No it's fine, if you like them let's keep them.

mruberry

Another awesome PR, @IvanYashchuk.

There are two very minor comments and one more significant comment about the examples using rcond. I would really like to merge this ASAP, so maybe we just remove the rcond examples for now and come back to add them later?

…escription

IvanYashchuk · 2021-01-07T18:40:13Z

@mruberry I think we are ready to merge this PR.

mruberry · 2021-01-11T15:13:19Z

@IvanYashchuk would you just rebase to resolve the merge conflicts and ping me?

IvanYashchuk · 2021-01-11T15:50:10Z

@mruberry done.

facebook-github-bot

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

facebook-github-bot · 2021-01-12T15:11:18Z

@mruberry merged this pull request in 9384d31.

Added torch.linalg.pinv

fc302ba

IvanYashchuk added module: numpy Related to numpy support, and also numpy compatibility of our operators module: linear algebra Issues related to specialized linear algebra operations in PyTorch; includes matrix multiply matmul labels Nov 23, 2020

facebook-github-bot added the cla signed label Nov 23, 2020

IvanYashchuk mentioned this pull request Nov 23, 2020

torch.linalg in PyTorch 1.10 tracker #42666

Closed

4 tasks

pytorchbot added the open source label Nov 23, 2020

IvanYashchuk added 9 commits December 9, 2020 09:40

Merge remote-tracking branch 'upstream/master' into linalg-pinv

3eb8552

Removed duplicated tests

7dcb934

Added overloading for Tensor rcond

14a0c71

Added tests for Tensor rcond and fixed implementation for this case

dec1212

Removed fixed xfailing tests

3170b60

Use linalg_eigh instead of symeig

492b697

Added OpInfo-based tests for pinv and pinv hermitian=True

cae7cb2

Updated docs, rcond now can be a Tensor

94325ac

flake8

44d86c5

IvanYashchuk marked this pull request as ready for review December 9, 2020 20:55

IvanYashchuk requested a review from glaringlee as a code owner December 9, 2020 20:55

IvanYashchuk requested review from mruberry and removed request for glaringlee December 9, 2020 20:55

fix variant_test_name

727baff

H-Huang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Dec 10, 2020

mruberry reviewed Dec 15, 2020

View reviewed changes

aten/src/ATen/native/LinearAlgebra.cpp Show resolved Hide resolved

mruberry reviewed Dec 15, 2020

View reviewed changes

aten/src/ATen/native/LinearAlgebra.cpp Show resolved Hide resolved

mruberry reviewed Dec 15, 2020

View reviewed changes

aten/src/ATen/native/LinearAlgebra.cpp Show resolved Hide resolved

mruberry reviewed Dec 15, 2020

View reviewed changes

aten/src/ATen/native/LinearAlgebra.cpp Show resolved Hide resolved

IvanYashchuk added 3 commits December 16, 2020 18:14

Updated docs

ed067a8

Added more rcond examples

b8714d9

Expanded docstring for HermitianOpInfo

6eb2f86

IvanYashchuk requested a review from mruberry December 16, 2020 19:03

IvanYashchuk added 8 commits December 18, 2020 15:21

Merge remote-tracking branch 'upstream/master' into linalg-pinv

f5fd672

Use 'use_c10_dispatcher: hacky_wrapper_for_legacy_signatures'

80be298

Fix CI failure: Torch not compiled with CUDA enabled

fa57cf3

Fix CI failure: Torch not compiled with CUDA enabled #2

76ec22b

Merge remote-tracking branch 'upstream/master' into linalg-pinv

c0045fc

Replace sample_inputs_pinverse with sample_inputs_linalg_pinv

42882c3

Merge remote-tracking branch 'upstream/master' into linalg-pinv

7c1672a

svd does support automatic differentiation for outputs with complex d…

609e931

…type

mruberry reviewed Dec 22, 2020

View reviewed changes

torch/linalg/__init__.py Outdated Show resolved Hide resolved

mruberry reviewed Dec 22, 2020

View reviewed changes

mruberry approved these changes Dec 22, 2020

View reviewed changes

IvanYashchuk and others added 3 commits December 22, 2020 21:49

Changed torch.linalg.svd -> torch.svd; added svd reference in rcond d…

73f9dce

…escription

Merge branch 'master' into linalg-pinv

0507670

Restore removed code after merge

dbd65d4

Merge branch 'master' into linalg-pinv

48ac5dd

facebook-github-bot reviewed Jan 11, 2021

View reviewed changes

facebook-github-bot closed this in 9384d31 Jan 12, 2021

facebook-github-bot added the Merged label Jan 12, 2021

IvanYashchuk mentioned this pull request Mar 23, 2021

Added "Tensor tol" overload of torch.linalg.matrix_rank #54157

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added linalg.pinv #48399

Added linalg.pinv #48399

IvanYashchuk commented Nov 23, 2020 •

edited

dr-ci bot commented Nov 23, 2020 •

edited by facebook-github-bot

codecov bot commented Dec 10, 2020 •

edited

mruberry Dec 15, 2020

mruberry Dec 15, 2020

IvanYashchuk Dec 16, 2020

IvanYashchuk Dec 16, 2020

mruberry Dec 21, 2020

mruberry Dec 15, 2020

IvanYashchuk Dec 16, 2020

mruberry Dec 15, 2020

mruberry Dec 15, 2020

IvanYashchuk Dec 16, 2020

IvanYashchuk commented Dec 16, 2020

mruberry Dec 22, 2020

IvanYashchuk Dec 22, 2020

mruberry Dec 22, 2020

mruberry Dec 22, 2020

IvanYashchuk Dec 22, 2020

IvanYashchuk Jan 5, 2021

mruberry Jan 5, 2021

mruberry left a comment

IvanYashchuk commented Jan 7, 2021

mruberry commented Jan 11, 2021

IvanYashchuk commented Jan 11, 2021

facebook-github-bot left a comment

facebook-github-bot commented Jan 12, 2021

Added linalg.pinv #48399

Added linalg.pinv #48399

Conversation

IvanYashchuk commented Nov 23, 2020 • edited

dr-ci bot commented Nov 23, 2020 • edited by facebook-github-bot

💊 CI failures summary and remediations

codecov bot commented Dec 10, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvanYashchuk commented Dec 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mruberry left a comment

Choose a reason for hiding this comment

IvanYashchuk commented Jan 7, 2021

mruberry commented Jan 11, 2021

IvanYashchuk commented Jan 11, 2021

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Jan 12, 2021

IvanYashchuk commented Nov 23, 2020 •

edited

dr-ci bot commented Nov 23, 2020 •

edited by facebook-github-bot

codecov bot commented Dec 10, 2020 •

edited