symeig for variables #2026

ncullen93 · 2017-07-09T20:12:42Z

this implements the symeig function for variables... I grabbed the gradient implementation directly from theano ... And I test it in this gist to show that it gets the same gradient as the theano version and that it passes gradcheck. I've also used this in actual networks so I'm fairly confident it's correct.

I obviously need to add actual tests, so any direction for that and anything else is much appreciated.

torch/autograd/_functions/linalg.py

+        x, w, v, = ctx.saved_variables
+
+        # gives an error if I don't do this..
+        x = x.data


torch/autograd/_functions/linalg.py

+            tri1 = lambda a: torch.triu(a, 1)
+
+        def G(n):
+            return sum([v[:, m] * grad_v.t()[n].matmul(v[:, m]) / (w[n] - w[m])


torch/autograd/_functions/linalg.py

+            return sum([v[:, m] * grad_v.t()[n].matmul(v[:, m]) / (w[n] - w[m])
+                       for m in range(N) if m != n])
+
+        g = sum([torch.ger(v[:, n], v[:, n] * grad_w[n] + G(n))


fmassa

Thanks for the PR! Do you think it would be possible to get eig as well?
I made some comments that I think need to be addressed before this PR can be merged. You could get the template for the tests from the gesv or inverse functions (I can give them to you tomorrow if needed, typing from the phone)

torch/autograd/_functions/linalg.py

+
+    @staticmethod
+    def forward(ctx, input, eigenvectors=False, upper=True):
+        ctx.eigenvectors = eigenvectors


ncullen93 · 2017-07-13T18:42:26Z

Major questions right now:

What to do if eigenvectors=False. I think an exception should be raised, because it's not clear how to compute the gradient without it.
Why do I have to call .data on x, w, and v? Am I saving those for the backend correctly?

Also, standard eig doesn't have implemented gradient in Theano... Neither does tensorflow. Both only have symmetric version. so I'll check around the literature but seems unlikely right now.

formatting for pyflakes

apaszke · 2017-07-13T19:14:00Z

You should probably override the flag and save the result for backward.
They're saved correctly, but something must be wrong with your backward implementation. What error do you get when you don't unpack the variables?

apaszke · 2017-07-13T19:14:31Z

Ad 2. You can use @once_differentiable and use ctx.saved_tensors. It will work, but won't be differentiable twice

small typo

removed `.data` call (not needed) and renamed `symeig` return values to `e` and `V` instead of `w` and `x` to be consistent with pytorch documentation.

ncullen93 · 2017-07-13T20:31:08Z

Re: 1. Ok, great that's a good solution. I will work on that.

Re: 2. Well I just tested it again - in the codebase, it only works without .data. However, implemented stand-alone it only works with .data. [EDIT]: Ah it's because in the docs stand-alone example, you use saved_tensors and in the backend you use saved_variables. It think it's all good now.

apaszke · 2017-07-13T23:05:35Z

It seems that the only problem is that you're mixing tensors with Variables, so just making sure that you keep everything wrapped would be enough. Add a couple of prints and see why do they get mixed up

Summary: Partially fixes #6890. (backward pass for non-symmetric eigen-decomposition is not implemented in other packages, e.g. autograd, mxnet, tensorflow, presumably because the eigenvalues can be imaginary for the general case, and AFAIK we cannot support complex numbers). This patch adds a backward function for the symmetric eigen-decomposition function `torch.symeig`. The formula used is taken from [here](http://eprints.maths.ox.ac.uk/1079/1/NA-08-01.pdf). Unit tests are added to verify correctness. There is still one outstanding issue, which is how to handle the case where the `symeig` is called with `eigenvectors=False`. In this case, the eigenvectors are returned as a zero tensor, but the backward computation for the eigenvalues depends on the eigenvectors. There was a previous attempt to implement this in #2026, where apaszke mentioned that the `eigenvectors` argument should be overridden so that they are saved for the backwards pass. The forward code is autogenerated, though, and it isn't clear to me how that would be done. I'd appreciate any guidance. For now, there is a unit test that will fail until that issue is resolved. Pull Request resolved: #8586 Reviewed By: ezyang Differential Revision: D8872760 Pulled By: SsnL fbshipit-source-id: 76614495d0f9c118fec163a428f32e5480b4d115

Summary: Partially fixes pytorch#6890. (backward pass for non-symmetric eigen-decomposition is not implemented in other packages, e.g. autograd, mxnet, tensorflow, presumably because the eigenvalues can be imaginary for the general case, and AFAIK we cannot support complex numbers). This patch adds a backward function for the symmetric eigen-decomposition function `torch.symeig`. The formula used is taken from [here](http://eprints.maths.ox.ac.uk/1079/1/NA-08-01.pdf). Unit tests are added to verify correctness. There is still one outstanding issue, which is how to handle the case where the `symeig` is called with `eigenvectors=False`. In this case, the eigenvectors are returned as a zero tensor, but the backward computation for the eigenvalues depends on the eigenvectors. There was a previous attempt to implement this in pytorch#2026, where apaszke mentioned that the `eigenvectors` argument should be overridden so that they are saved for the backwards pass. The forward code is autogenerated, though, and it isn't clear to me how that would be done. I'd appreciate any guidance. For now, there is a unit test that will fail until that issue is resolved. Pull Request resolved: pytorch#8586 Reviewed By: ezyang Differential Revision: D8872760 Pulled By: SsnL fbshipit-source-id: 76614495d0f9c118fec163a428f32e5480b4d115

zou3519 · 2018-07-31T16:49:28Z

I think this is superceded by #8586; please reopen if I am incorrect.

Summary: Partially fixes pytorch#6890. (backward pass for non-symmetric eigen-decomposition is not implemented in other packages, e.g. autograd, mxnet, tensorflow, presumably because the eigenvalues can be imaginary for the general case, and AFAIK we cannot support complex numbers). This patch adds a backward function for the symmetric eigen-decomposition function `torch.symeig`. The formula used is taken from [here](http://eprints.maths.ox.ac.uk/1079/1/NA-08-01.pdf). Unit tests are added to verify correctness. There is still one outstanding issue, which is how to handle the case where the `symeig` is called with `eigenvectors=False`. In this case, the eigenvectors are returned as a zero tensor, but the backward computation for the eigenvalues depends on the eigenvectors. There was a previous attempt to implement this in pytorch#2026, where apaszke mentioned that the `eigenvectors` argument should be overridden so that they are saved for the backwards pass. The forward code is autogenerated, though, and it isn't clear to me how that would be done. I'd appreciate any guidance. For now, there is a unit test that will fail until that issue is resolved. Pull Request resolved: pytorch#8586 Reviewed By: ezyang Differential Revision: D8872760 Pulled By: SsnL fbshipit-source-id: 76614495d0f9c118fec163a428f32e5480b4d115

symeig for variables

41962c8

fmassa reviewed Jul 11, 2017

View reviewed changes

torch/autograd/_functions/linalg.py Outdated

x, w, v, = ctx.saved_variables

# gives an error if I don't do this..

x = x.data

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

fmassa reviewed Jul 11, 2017

View reviewed changes

torch/autograd/_functions/linalg.py Outdated

tri1 = lambda a: torch.triu(a, 1)

def G(n):

return sum([v[:, m] * grad_v.t()[n].matmul(v[:, m]) / (w[n] - w[m])

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

fmassa reviewed Jul 11, 2017

View reviewed changes

torch/autograd/_functions/linalg.py Outdated

return sum([v[:, m] * grad_v.t()[n].matmul(v[:, m]) / (w[n] - w[m])

for m in range(N) if m != n])

g = sum([torch.ger(v[:, n], v[:, n] * grad_w[n] + G(n))

This comment was marked as off-topic.

Sign in to view

fmassa requested changes Jul 11, 2017

View reviewed changes

torch/autograd/_functions/linalg.py

@staticmethod

def forward(ctx, input, eigenvectors=False, upper=True):

ctx.eigenvectors = eigenvectors

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

Update linalg.py

77f963f

formatting for pyflakes

NC Cullen added 2 commits July 13, 2017 16:19

Update variable.py

dff11d0

small typo

Update linalg.py

8f44e07

removed `.data` call (not needed) and renamed `symeig` return values to `e` and `V` instead of `w` and `x` to be consistent with pytorch documentation.

sethah mentioned this pull request Jun 17, 2018

Implement backward for torch.symeig #8586

Closed

zou3519 closed this Jul 31, 2018

ezyang added the open source label Jun 24, 2019

symeig for variables #2026

symeig for variables #2026

Uh oh!

Conversation

ncullen93 commented Jul 9, 2017 • 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!

fmassa 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!

ncullen93 commented Jul 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apaszke commented Jul 13, 2017

Uh oh!

apaszke commented Jul 13, 2017

Uh oh!

ncullen93 commented Jul 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apaszke commented Jul 13, 2017

Uh oh!

zou3519 commented Jul 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ncullen93 commented Jul 9, 2017 •

edited

Loading

ncullen93 commented Jul 13, 2017 •

edited

Loading

ncullen93 commented Jul 13, 2017 •

edited

Loading