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

[feature request] conjugate transpose operator #45063

Closed
nikitaved opened this issue Sep 21, 2020 · 26 comments
Closed

[feature request] conjugate transpose operator #45063

nikitaved opened this issue Sep 21, 2020 · 26 comments
Assignees
Labels
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

@nikitaved
Copy link
Collaborator

nikitaved commented Sep 21, 2020

🚀 Feature

As per title

Motivation

Much needed for backward methods with complex support in forward.

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

@nikitaved nikitaved added 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 labels Sep 21, 2020
@ezyang
Copy link
Contributor

ezyang commented Sep 21, 2020

We can (and should) add a hermitian operator; but I'm pretty unconvinced that we should silently assume the user meant H when they say T, in contravention of mathematics.

@ezyang
Copy link
Contributor

ezyang commented Sep 21, 2020

cc @boeddeker

@boeddeker
Copy link
Contributor

I think most people know numpy.
In numpy the transpose function does only transpose (Beside doing slightly different things).

When reading the literature, many people say "conjugate transpose" (e.g. [1]), so implementing the transpose operation to do also a conjugate, it would lead to confusion.

A hermitian operator could help to reduce the overhead to zero for complex gradients, when they are used for real data.
But, I am not sure, how relevant this overhead is.

[1] https://en.wikipedia.org/wiki/Conjugate_transpose

@nikitaved
Copy link
Collaborator Author

nikitaved commented Sep 21, 2020

Tensorflow allows conjugation in transpose, although it does not do that by default.
In inner-product spaces (with L2 norm) over reals T is essentially an adjoint operator, so is H over complex numbers.
Then, what do you think about a method like adjoint? In most backward implementations T exactly is adjoint.
I agree, transpose has to be independent, because of, for example, matrix products.

@nikitaved nikitaved changed the title transpose doing conjugation for complex input by default [feature requiest] conjugate transpose operator Sep 21, 2020
@nikitaved nikitaved changed the title [feature requiest] conjugate transpose operator [feature request] conjugate transpose operator Sep 21, 2020
@anjali411
Copy link
Contributor

anjali411 commented Sep 21, 2020

I think most people know numpy.
In numpy the transpose function does only transpose (Beside doing slightly different things).

When reading the literature, many people say "conjugate transpose" (e.g. [1]), so implementing the transpose operation to do also a conjugate, it would lead to confusion.

I agree with @boeddeker here. I think we should follow the numpy semantics and only perform conjugate transpose when user calls tensor.H.

As Ed pointed out, I think tensorflow's behavior to return conjugate transpose for torch.transpose is unintuitive mathematically.

@pearu
Copy link
Collaborator

pearu commented Sep 21, 2020

transpose (or T, in short) should do transpose only.

For conjugate transpose, I suggest introducing the method transjugate (or H, in short) as in https://en.wikipedia.org/wiki/Conjugate_transpose

Adjoint is specific to square matrices.

@malfet malfet added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 22, 2020
@boeddeker
Copy link
Contributor

My favorite is hermitian and .H.

  • .H is often used in formulas and would be the counterpart to .T.
  • hermitian starts with the same letter as .H. To it is easy to see the connection.
  • transjugate (Beside @t-vi comment): The name is to close to transpose.
  • adjoint sound very similar to adjungate.

@IvanYashchuk
Copy link
Collaborator

IvanYashchuk commented Oct 2, 2020

In Julia, the conjugate transposition operation is called adjoint and it's not specific to square matrices.

@nikitaved
Copy link
Collaborator Author

nikitaved commented Oct 2, 2020

If Hermitian is adopted, then what do you think about other methods, like symeig? It does support Hermitian input as of now, but it is not obvious given its name. Do we keep the names and support complex input for them, or do we rather make aliases for complex inputs, like symeig becomes hermeig or heig?

@nikitaved
Copy link
Collaborator Author

nikitaved commented Oct 2, 2020

Exactly, I would not expect symeig work for complex operators, unless they are self-adjoint. But if symeig stays for Hermitian inputs, then why not make transpose support conjugation? ;)

@boeddeker
Copy link
Contributor

then why not make transpose support conjugation

I think support would be ok, but not by default.
So when translating conjugate transpose to code as .transpose(..., conj=True), it is obvious, what it does (but maybe not really better than .transpose(...).conj())

@nikitaved
Copy link
Collaborator Author

conj is no-op for non-complex input, so, probably, it is fine to just use conj and be happy with it.

@pearu
Copy link
Collaborator

pearu commented Oct 2, 2020

From a general perspective, .transpose(..., conj=True) indicates that a matrix operation (transpose in this case) can be attributed with an element-wise unary operation (conjugate in this case).
Using the same pattern, one could have .transpose(.., negate=True), etc.
I wonder if people would be willing to accept and extend this pattern?

While transjugate is not widely used in terms of google hits, the term has a unique meaning and has been documented by independent sources (google: transjugate) to mean conjugate transpose operation.

@hameerabbasi
Copy link
Collaborator

My favorite is hermitian and .H.

I like hermitian, .H implies an O(1) operation as it is a property, which this isn't. For this reason, I don't like it.

@rgommers
Copy link
Collaborator

rgommers commented Oct 2, 2020

.H implies an O(1) operation as it is a property, which this isn't.

This is also the rationale for why numpy never added .H.

I'd go with .hermitian() or .adjoint(), both seem like sensible names.

@nikitaved
Copy link
Collaborator Author

nikitaved commented Oct 2, 2020

Hermitian could be confusing in my opinion as it has the meaning akin symmetric for real matrices.

@ezyang
Copy link
Contributor

ezyang commented Oct 5, 2020

A few things:

  1. If conjugate views <https://github.com/pytorch/pytorch/pull/44799>_ go in, hermitian will be O(1). So we can (and should) provide a .H attribute.
  2. I don't think we, as framework developers, should really be in the business of telling mathematicians what terms are and aren't ambiguous. So if a term like "hermitian" is standard nomenclature in the mathematical community, we should support it. (I also happen to think that "taking the Hermitian of a matrix" is pretty unambiguous, since the other property is a description of a matrix, rather than an action you take on it.
  3. There isn't really any reason we can't provide multiple aliases for an operation; it's not an all or nothing deal. Though, granted, this is not that Pythonic (there is one obvious way to do it)
  4. "transpose" and "symeig" are not analogous situations. It is very reasonable to want to do a non-conjugate transpose on a matrix. But it makes very little sense to do a "symeig" on a symmetric but non-hermitian complex matrix, this is a very unnatural thing to do and won't give you real eigenvalues (which means you will have trouble with dtypes anyway). That being said, we should provide "eigh" (the Numpy naming) to disambiguate this situation.
  5. Hermitian (operator), the verb, is not specific to square matrices (it is only the adjective, Hermitian, that is applicable to square matrices only)

@anjali411
Copy link
Contributor

From the discussion, it seems like the general consensus is to call the conjugate transpose operator .hermitian() in PyTorch. If we end up adding conjugate views, I think we should add tensor.H as a follow-up which would be a view (O(1) operation).

@lezcano
Copy link
Collaborator

lezcano commented Feb 1, 2021

My two cents on this:

I completely agree with the X.H attribute and the O(1) operation discussion.

On the naming of the full operation, I prefer adjoint over hermitian for the following reasons:

I think that "taking the Hermitian of a matrix" is a somewhat odd thing to say. I have personally never encountered it in maths writings. If I read X.hermitian(), I would think that it's a boolean operation that is equivalent to the code X == X.H.

In this sense, the term adjoint is more general and offers no confusion. It is clear that, in the real case, it refers to $X^T$ and in the complex case it refers to $X^H$. It is also pretty common to see the phrase "taking adjoints" in mathematical papers. At the end of the day, all the backward methods do is to compute the adjoint of the linearisation (differential) of each map. This gives the name for the backprop algorithm in mathematics: The adjoint method.

@IvanYashchuk
Copy link
Collaborator

Here's an incomplete list how this operation is called in different popular programs:
adjoint(): Eigen, TensorFlow, Julia
conjugate_transpose(): Wolfram Mathematica
ctranspose(): Matlab
hermitian_transpose(): Maple
H(): R
hermitian(): -

@anjali411
Copy link
Contributor

anjali411 commented Mar 10, 2021

I think that "taking the Hermitian of a matrix" is a somewhat odd thing to say. I have personally never encountered it in maths writings. If I read X.hermitian(), I would think that it's a boolean operation that is equivalent to the code X == X.H.

sold.

Here's an incomplete list how this operation is called in different popular programs:

adjoint and conjugate_transpose both seem like reasonable options however I prefer the first over the other since it's less wordy.

@lezcano lezcano self-assigned this Aug 20, 2021
lezcano added a commit that referenced this issue Sep 3, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 3, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 6, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 6, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 21, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 21, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 21, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 21, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 21, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 21, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 23, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 23, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 28, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 28, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 28, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Sep 28, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Oct 11, 2021
This PR follows the discussion in
#45063 (comment)

Fixes #45063

ghstack-source-id: d469234318e753ca0f97823fb4c26a3dfa0676cc
Pull Request resolved: #64179
lezcano added a commit that referenced this issue Oct 11, 2021
… properties"


This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
lezcano added a commit that referenced this issue Oct 11, 2021
This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry @lezcano nikitaved rgommers pmeier asmeurer leofang @AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Differential Revision: [D30730483](https://our.internmc.facebook.com/intern/diff/D30730483)

[ghstack-poisoned]
wconstab pushed a commit that referenced this issue Oct 20, 2021
Summary:
Pull Request resolved: #64179

This PR follows the discussion in #45063 (comment)

Fixes #45063

cc ezyang anjali411 dylanbespalko mruberry Lezcano nikitaved rgommers pmeier asmeurer leofang AnirudhDagar asi1024 emcastillo kmaehashi heitorschueroff

Test Plan: Imported from OSS

Reviewed By: bertmaher

Differential Revision: D30730483

Pulled By: anjali411

fbshipit-source-id: 821d25083f5f682450f6812bf852dc96a1cdf9f2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

10 participants