Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

torch.eig should return complex tensor #43081

Closed
Tracked by #33152
zasdfgbnm opened this issue Aug 14, 2020 · 5 comments
Closed
Tracked by #33152

torch.eig should return complex tensor #43081

zasdfgbnm opened this issue Aug 14, 2020 · 5 comments
Labels
enhancement Not as big of a feature, but technically not a bug. Should be easy to fix module: bc-breaking Related to a BC-breaking change module: complex Related to complex number support in PyTorch module: linear algebra Issues related to specialized linear algebra operations in PyTorch; includes matrix multiply matmul triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@zasdfgbnm
Copy link
Collaborator

zasdfgbnm commented Aug 14, 2020

Currently the return value of torch.eig is:

eigenvalues (Tensor):

Shape (n \times 2)(n×2) . Each row is an eigenvalue of input, where the first element is
the real part and the second element is the imaginary part. The eigenvalues are not
necessarily ordered.

eigenvectors (Tensor):

If eigenvectors=False, it’s an empty tensor. Otherwise, this
tensor of shape (n \times n)(n×n) can be used to compute normalized (unit length)
eigenvectors of corresponding eigenvalues as follows. If the corresponding eigenvalues[j]
is a real number, column eigenvectors[:, j] is the eigenvector corresponding to eigenvalues[j].
If the corresponding eigenvalues[j] and eigenvalues[j + 1] form a complex conjugate pair,
then the true eigenvectors can be computed as
\text{true eigenvector}[j] = eigenvectors[:, j] + i \times eigenvectors[:, j + 1]true eigenvector[j]=eigenvectors[:,j]+i×eigenvectors[:,j+1] ,
\text{true eigenvector}[j + 1] = eigenvectors[:, j] - i \times eigenvectors[:, j + 1]true eigenvector[j+1]=eigenvectors[:,j−i×eigenvectors[:,j+1] .

This behavior is because of historical reason when complex tensors was not a thing. Since complex tensor is now supported,
the return value of eigenvalues should then be a complex tensor of shape (n,), and eigenvectors should also be a complex tensor of shape (n,n) where eignvectors[:,j] is the jth eigen vector.

cc @ezyang @anjali411 @dylanbespalko @mruberry @gchanan @vincentqb @vishwakftw @jianyuh @nikitaved @pearu

@izdeby izdeby added enhancement Not as big of a feature, but technically not a bug. Should be easy to fix module: complex Related to complex number support in PyTorch module: operators triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Aug 14, 2020
@mrshenli mrshenli self-assigned this Oct 2, 2020
@mrshenli
Copy link
Contributor

mrshenli commented Oct 2, 2020

self-assigned to reserve for bootcamp

@ezyang ezyang added the module: bc-breaking Related to a BC-breaking change label Oct 5, 2020
@ezyang
Copy link
Contributor

ezyang commented Oct 5, 2020

@mrshenli I don't think this is appropriate for bootcamp, as this change would have BC implications

@mruberry mruberry added module: linear algebra Issues related to specialized linear algebra operations in PyTorch; includes matrix multiply matmul and removed module: operators labels Oct 6, 2020
@mruberry
Copy link
Collaborator

mruberry commented Oct 6, 2020

Instead of changing the behavior of torch.eig in a BC-breaking way, I suggest we just implement torch.linalg.eig properly.

@mrshenli mrshenli removed their assignment Oct 9, 2020
@mrshenli
Copy link
Contributor

mrshenli commented Oct 9, 2020

unassigned myself, this is no longer reserved for bootcamp. Please feel free to grab if you are interested.

@mruberry
Copy link
Collaborator

Closing because we aren't going to change the behavior of torch.eig, but plan to implement the correct behavior with torch.linalg.eig.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Not as big of a feature, but technically not a bug. Should be easy to fix module: bc-breaking Related to a BC-breaking change module: complex Related to complex number support in PyTorch module: linear algebra Issues related to specialized linear algebra operations in PyTorch; includes matrix multiply matmul triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

No branches or pull requests

5 participants