[ready] Add matrix_power #10068

vishwakftw · 2018-07-31T11:52:04Z

Tests added
Doc string added

1. Tests added 2. Doc string added TODO: 3. Derivative for n <= 0

aten/src/ATen/native/LinearAlgebra.cpp

+    }
+    return identities;
+  } else if (n < 0) {
+    AT_CHECK(a.dim() == 2, "Negative powers for batch matrices are currently not supported")


aten/src/ATen/native/LinearAlgebra.cpp


+Tensor matrix_power(const Tensor& a, int64_t n) {
+  AT_CHECK(a.dim() >= 2 && at::isFloatingType(a.type().scalarType()),
+           "pinverse(", a.type(), "{", a.sizes(), "}): expected a tensor "


aten/src/ATen/native/LinearAlgebra.cpp

+
+  Tensor result, z;
+  int64_t r;
+  while (n > 0) {


aten/src/ATen/native/LinearAlgebra.cpp

+    return identities;
+  } else if (n < 0) {
+    AT_CHECK(a.dim() == 2, "Negative powers for batch matrices are currently not supported")
+    Tensor a_ = at::native::inverse(a);


aten/src/ATen/native/LinearAlgebra.cpp

+  AT_CHECK(a.dim() >= 2 && at::isFloatingType(a.type().scalarType()),
+           "matrix_power(", a.type(), "{", a.sizes(), "}): expected a tensor "
+           "of floating types with dim at least 2");
+  if (n == 0) {


vishwakftw · 2018-08-03T17:49:43Z

This is ready for review.

vishwakftw · 2018-08-10T15:16:32Z

@fmassa Could you review this please?

yf225 · 2018-08-14T16:35:39Z

@fmassa Ping :)

aten/src/ATen/native/LinearAlgebra.cpp

+           "matrix_power(", a.type(), "{", a.sizes(), "}): expected a tensor "
+           "of floating types with dim at least 2");
+  if (n == 0) {
+    return a.clone().copy_(at::eye(a.size(-2), a.options()).expand_as(a));


fmassa

Apart from the issue in the n==0 case, everything else looks good to me

…input tensor from the autograd graph This reverts commit 32fd6f7.

facebook-github-bot

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

yf225 · 2018-08-21T16:42:15Z

@pytorchbot retest this please

yf225 · 2018-08-21T16:44:40Z

@vishwakftw I just retriggered the CI to see if the ASAN test failure is consistent, if so we probably should look at how to fix it or skip it.

vishwakftw · 2018-08-22T07:52:34Z

Build failure seems to be unrelated to the PR.

vishwakftw · 2018-08-27T16:59:55Z

@yf225 is this good to go? Sorry about the reminder.

facebook-github-bot

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

test/test_torch.py

+
+        # Single matrix, but full rank
+        # This is for negative powers
+        from test_autograd import random_fullrank_matrix_distinct_singular_value


vishwakftw · 2018-08-29T17:04:12Z

@yf225 could you try importing now? I am sorry if you are busy with other tasks - thought I should send a reminder just in case.

facebook-github-bot

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

vishwakftw · 2018-09-03T02:05:28Z

@yf225 is this good to go?

yf225 · 2018-09-04T05:13:12Z

@vishwakftw Apologies for the delay - there are some internal dependency errors that I am still trying to figure out how to fix. I will have an update here as soon as possible.

vishwakftw · 2018-09-04T05:14:07Z

No worries @yf225 . Thank you for helping out.

yf225 · 2018-09-05T02:40:13Z

@vishwakftw Sorry for the delay on this - I think your original idea of putting from test_autograd import random_fullrank_matrix_distinct_singular_value in _test_matrix_power is actually the correct one, as it prevents other tests that depend on test_torch.py to also depend on test_autograd.py. Do you mind reverting the commit for this (dcf13e5) and I can push it through internally? Thanks a lot and sorry for the confusion on this one.

vishwakftw · 2018-09-05T03:04:23Z

No problem @yf225. I will revert the previous commit right away. Thank you again for helping out with this.

This reverts commit dcf13e5.

facebook-github-bot

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

vishwakftw · 2018-09-08T03:34:34Z

@yf225 did the internal tests pass?

vishwakftw · 2018-09-08T07:23:42Z

Closing in favour of #11421

Summary: vishwakftw Your patch needed some updates because the default native function dispatches changed from `[function, method]` to `[function]`. The CI was run before that change happened so it still shows green, but the internal test caught it. I did some changes when rebasing and updating so I didn't just force push to your branch. Let's see if this passes CI and internal test. If it does, let me know if you want me to force push to your branch or use this PR instead. Note to reviewers: patch was already approved at #10068 . cc yf225 Pull Request resolved: #11421 Differential Revision: D9733407 Pulled By: SsnL fbshipit-source-id: cf2ed293bb9942dcc5158934ff4def2f63252599

Summary: vishwakftw Your patch needed some updates because the default native function dispatches changed from `[function, method]` to `[function]`. The CI was run before that change happened so it still shows green, but the internal test caught it. I did some changes when rebasing and updating so I didn't just force push to your branch. Let's see if this passes CI and internal test. If it does, let me know if you want me to force push to your branch or use this PR instead. Note to reviewers: patch was already approved at pytorch#10068 . cc yf225 Pull Request resolved: pytorch#11421 Differential Revision: D9733407 Pulled By: SsnL fbshipit-source-id: cf2ed293bb9942dcc5158934ff4def2f63252599

Add matrix_power

df8a474

1. Tests added 2. Doc string added TODO: 3. Derivative for n <= 0

vishwakftw requested review from apaszke, colesbury, ezyang, gchanan, soumith, ssnl, zdevito and zou3519 as code owners July 31, 2018 11:52

vishwakftw commented Jul 31, 2018

View reviewed changes

aten/src/ATen/native/LinearAlgebra.cpp Outdated

}

return identities;

} else if (n < 0) {

AT_CHECK(a.dim() == 2, "Negative powers for batch matrices are currently not supported")

This comment was marked as off-topic.

Sign in to view

fmassa reviewed Jul 31, 2018

View reviewed changes

CR

9c6bbcc

vishwakftw force-pushed the matrix-power branch from 4e6dd5b to 9c6bbcc Compare July 31, 2018 12:28

vishwakftw commented Jul 31, 2018

View reviewed changes

at::native::inverse --> at::inverse

2d1b1e2

vishwakftw commented Aug 1, 2018

View reviewed changes

vishwakftw added 3 commits August 2, 2018 08:47

Modify tests for negative powers

047e799

Exclude matrix_power for negative powers in test_jit

f4ab2bb

Use copy_ for n = 0

fedd3b2

Minor changes

4abebd4

vishwakftw changed the title ~~Add matrix_power~~ [ready] Add matrix_power Aug 4, 2018

soumith suggested changes Aug 15, 2018

View reviewed changes

Fix redundant copy.

32fd6f7

soumith approved these changes Aug 16, 2018

View reviewed changes

fmassa reviewed Aug 16, 2018

View reviewed changes

vishwakftw added 2 commits August 17, 2018 08:29

Revert "Fix redundant copy." Doing this to prevent detachment of the …

852b4f0

…input tensor from the autograd graph This reverts commit 32fd6f7.

Merge branch 'master' into matrix-power

ce6b2a8

facebook-github-bot reviewed Aug 21, 2018

View reviewed changes

vishwakftw added 3 commits August 22, 2018 07:38

fix rocm tests

e0d1a2a

fix typo

a534433

fix test_autograd

e645a97

Merge branch 'master' into matrix-power

49903a8

facebook-github-bot reviewed Aug 27, 2018

View reviewed changes

Merge branch 'master' into matrix-power

8bd6456

yf225 reviewed Aug 28, 2018

View reviewed changes

test/test_torch.py

# Single matrix, but full rank

# This is for negative powers

from test_autograd import random_fullrank_matrix_distinct_singular_value

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

Move import to top

dcf13e5

facebook-github-bot reviewed Aug 29, 2018

View reviewed changes

Revert "Move import to top"

b1af533

This reverts commit dcf13e5.

facebook-github-bot reviewed Sep 5, 2018

View reviewed changes

ssnl mentioned this pull request Sep 8, 2018

Add matrix power #11421

Closed

vishwakftw closed this Sep 8, 2018

vishwakftw deleted the matrix-power branch September 10, 2018 05:02

ezyang added the open source label Jun 24, 2019

[ready] Add matrix_power #10068

[ready] Add matrix_power #10068

Uh oh!

Conversation

vishwakftw commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

vishwakftw commented Aug 3, 2018

Uh oh!

vishwakftw commented Aug 10, 2018

Uh oh!

yf225 commented Aug 14, 2018

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

fmassa left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

yf225 commented Aug 21, 2018

Uh oh!

yf225 commented Aug 21, 2018

Uh oh!

vishwakftw commented Aug 22, 2018

Uh oh!

vishwakftw commented Aug 27, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

vishwakftw commented Aug 29, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

vishwakftw commented Sep 3, 2018

Uh oh!

yf225 commented Sep 4, 2018

Uh oh!

vishwakftw commented Sep 4, 2018

Uh oh!

yf225 commented Sep 5, 2018

Uh oh!

vishwakftw commented Sep 5, 2018

vishwakftw commented Jul 31, 2018 •

edited

Loading