Skip to content

Conversation

@anjali411
Copy link
Contributor

@anjali411 anjali411 commented May 27, 2020

Stack from ghstack:

Differential Revision: D22057886

This PR adds the following:

  1. view_as_real, view_as_complex for complex tensors
  2. Adds the replay logic for these complex views that cause a change in the metadata (in this case- dtype):
    as_strided doesn't carry the tensor metadata information so this PR adds a replay logic similar to what XLA does for these view functions that cause a change in metadata. There are four possible scenarios concerning this PR:
  3. view function that uses as_strided followed by a view function that uses replay (example. .transpose(...) followed by view_as_real)
  4. view function that uses replay followed by a view function that uses as_strided (example. view_as_real followed by .transpose(...))
  5. view function that uses replay followed by another view function that uses replay (example. view_as_real followed by view_as_complex)
  6. view function that uses as_strided followed by another view function that uses as_strided (this is what always happens for non-xla tensors rn)

this covers case 1: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152R130

this covers case 2: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152R154

Case 3. is handled by existing XLA logic here: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152L120

anjali411 added a commit that referenced this pull request May 27, 2020
ghstack-source-id: c3c3bf9
Pull Request resolved: #39099
@dr-ci
Copy link

dr-ci bot commented May 27, 2020

💊 CI failures summary and remediations

As of commit 93e3cf5 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 238 times.

[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
@anjali411 anjali411 requested review from albanD and apaszke as code owners May 28, 2020 15:38
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request May 28, 2020
ghstack-source-id: d745ee2
Pull Request resolved: #39099
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request May 28, 2020
ghstack-source-id: 1bddc13
Pull Request resolved: #39099
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request May 29, 2020
ghstack-source-id: 6a406b0
Pull Request resolved: #39099
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 1, 2020
ghstack-source-id: 6e4363a
Pull Request resolved: #39099
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 2, 2020
ghstack-source-id: 8d44b3e
Pull Request resolved: #39099
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 2, 2020
ghstack-source-id: eb564ad
Pull Request resolved: #39099
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 3, 2020
ghstack-source-id: 421dfd9
Pull Request resolved: #39099
[WIP]
TODO: 
1. add documentation
2. add tests
3. autograd code

[ghstack-poisoned]
x.real.sum().backward()
self.assertEqual(x.grad, torch.ones_like(x))

# remove this test after gradcheck support is added for non-holomoprphic functions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about a test where you manipulate both the real and imag parts and then backward through both those results?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a case that this test covers which hasn't already been covered


setattr(TestAutogradDeviceType, test_name, do_test)

class TestAutogradComplex(TestCase):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this class be made device generic so it runs on CPU and CUDA?

Copy link
Contributor Author

@anjali411 anjali411 Jun 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ithink autograd tests run for both cpu and cuda already

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests won't run on both but that's OK for now.

@anjali411 anjali411 requested a review from ezyang June 19, 2020 21:14
@anjali411 anjali411 requested review from albanD and ezyang June 22, 2020 02:55
Differential Revision: [D22057886](https://our.internmc.facebook.com/intern/diff/D22057886)

This PR adds the following:
1. `view_as_real`, `view_as_complex` for complex tensors
2. Adds the replay logic for these complex views that cause a change in the metadata (in this case- dtype):
`as_strided` doesn't carry the tensor metadata information so this PR adds a replay logic similar to what `XLA` does for these view functions that cause a change in metadata. There are four possible scenarios concerning this PR:
1.  view function that uses as_strided followed by a view function that uses replay (example. `.transpose(...)` followed by `view_as_real`)
2. view function that uses replay followed by a view function that uses as_strided (example. `view_as_real` followed by `.transpose(...)`)
3. view function that uses replay followed by another view function that uses replay (example. `view_as_real` followed by `view_as_complex`)
4.  view function that uses as_strided followed by another view function that uses as_strided (this is what always happens for non-xla tensors rn)

this covers case 1: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152R130

this covers case 2: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152R154

Case 3. is handled by existing XLA logic here: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152L120



[ghstack-poisoned]
@anjali411 anjali411 requested a review from mruberry June 22, 2020 17:21
@mruberry mruberry removed the request for review from apaszke June 22, 2020 18:32
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

Just remember to update docs/source/torch.rst.

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

This PR adds the following:
1. `view_as_real`, `view_as_complex` for complex tensors
2. Adds the replay logic for these complex views that cause a change in the metadata (in this case- dtype):
`as_strided` doesn't carry the tensor metadata information so this PR adds a replay logic similar to what `XLA` does for these view functions that cause a change in metadata. There are four possible scenarios concerning this PR:
1.  view function that uses as_strided followed by a view function that uses replay (example. `.transpose(...)` followed by `view_as_real`)
2. view function that uses replay followed by a view function that uses as_strided (example. `view_as_real` followed by `.transpose(...)`)
3. view function that uses replay followed by another view function that uses replay (example. `view_as_real` followed by `view_as_complex`)
4.  view function that uses as_strided followed by another view function that uses as_strided (this is what always happens for non-xla tensors rn)

this covers case 1: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152R130

this covers case 2: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152R154

Case 3. is handled by existing XLA logic here: https://github.com/pytorch/pytorch/pull/39099/files#diff-88d45e2b30fab214a6aa074fd30eb152L120



[ghstack-poisoned]
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Mike for the update.
Good to go from my side !

@facebook-github-bot
Copy link
Contributor

@anjali411 merged this pull request in 8ec2ae9.

@facebook-github-bot facebook-github-bot deleted the gh/anjali411/32/head branch June 26, 2020 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: complex Related to complex number support in PyTorch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants