Decompose/add reference for `view_as_complex` #108005

Fidget-Spinner · 2023-08-26T12:11:51Z

Aten source:

pytorch/aten/src/ATen/native/ComplexHelper.h

Line 78 in d4a9963

Tensor view_as_complex(const Tensor& self) {

Documentation reference:
https://pytorch.org/docs/stable/generated/torch.view_as_complex.html

Note: this adds a new primitive view_of_dtype, which is trivially implemented, as its meta function is already implemented elsewhere.

Finally, this is not registered as a decomposition (yet), because TorchInductor does not yet support complex types. It should be added once we do.

Closes #108020 as well.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @ngimel @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov @lezcano

pytorch-bot · 2023-08-26T12:11:54Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/108005

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 93ce05d with merge base 9f37aec ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Fidget-Spinner · 2023-08-26T12:13:49Z

torch/_prims/__init__.py

@@ -172,7 +173,6 @@
    "item",
    "maximum_value",
    "minimum_value",
-    "to_dtype",


Removed this as this function doesn't exist.

peterbell10 · 2023-08-26T18:20:03Z

torch/_refs/__init__.py

+    new_storage_offset = self.storage_offset() // 2
+    if not utils.is_complex_dtype(input_dtype):
+        self = prims.view_of_dtype(self, utils.corresponding_complex_dtype(input_dtype))
+
+    return self.as_strided(new_sizes, new_strides, new_storage_offset)


This decomposition isn't ideal for inductor since it 1) goes directly to prims 2) calls as_strided. It would make more sense to decompose into aten.view.dtype.

Suggested change

new_storage_offset = self.storage_offset() // 2

if not utils.is_complex_dtype(input_dtype):

self = prims.view_of_dtype(self, utils.corresponding_complex_dtype(input_dtype))

return self.as_strided(new_sizes, new_strides, new_storage_offset)

complex_dtype = utils.corresponding_complex_dtype(input_dtype)

return self.view(complex_dtype)[..., 0]

Which you could in turn implement with prims.view_of_dtype.

Another reason to do it this way is to fix the meta registration for view.dtype which currently isn't a view and doesn't handle different sized dtypes at all!

pytorch/torch/_meta_registrations.py

Lines 2645 to 2647 in 808e088

@register_meta(aten.view.dtype)

def view_dtype(self, dtype):

return utils.clone_preserve_strides(self).to(dtype)

Alright. I think I will work on the decomposition for aten.view.dtype (which means i also need to fix the meta). Will do that in another PR. Thanks for the review!

peterbell10 · 2023-08-26T18:21:11Z

torch/_refs/__init__.py

@@ -918,6 +919,47 @@ def trunc(a):
    return prims.trunc(a)


+# TODO: register this as a real ref/decomposition once TorchInductor supports complex!


It should be fine to register the decomposition and inductor won't try to use it by default.

When I registered this as a decomposition, tests started failing and it seems Inductor generated code with complex64 and like.

Oh it is being picked up because you have it in the core_aten_decompositions list which is used by inductor here:

pytorch/torch/_inductor/decomposition.py

Line 64 in 78810d7

decompositions = {**core_aten_decompositions(), **inductor_decompositions}

The function is composite(explicit) and it's already symintified in core, so there's not much point in registering it as a decomposition, not now nor when we support complex.

@lezcano CompositeExplicitAutograd operators are not traced through by FakeTensor, so you still need a decomposition.

Any reason why we wouldn't want to trace through those? Just because we are assuming that there may be some in-place ops or smth?

Well in the case of view.dtype it calls raw TensorImpl methods and wouldn't materialize anything meaningful in the fx graph if we traced through it. Although I would question if that should be legal in composite methods. It certainly wouldn't work for any tensor subclasses so maybe should be registered as CPU, CUDA, Meta.

Either way I'm guessing this function isn't alone in being registered as CompositeExplicit but actually doesn't dispatch to anything.

torch/_prims/__init__.py

Fidget-Spinner · 2023-09-02T13:53:40Z

torch/_meta_registrations.py

@@ -2606,7 +2606,54 @@ def meta_complex(real, imag):

 @register_meta(aten.view.dtype)
 def view_dtype(self, dtype):


Aten source

pytorch/aten/src/ATen/native/TensorConversions.cpp

Line 737 in bad3f2d

Tensor view_dtype(const Tensor& self, ScalarType dtype) {

ezyang · 2023-09-02T14:03:03Z

As a low level view-y operation, my personal preference would have been to just symbolify the underlying C++ implementation

Co-Authored-By: peterbell10 <13238737+peterbell10@users.noreply.github.com>

Fidget-Spinner · 2023-09-02T14:41:24Z

As a low level view-y operation, my personal preference would have been to just symbolify the underlying C++ implementation

I see. Just to clarify, you mean so support SymInts in both self.view.dtype overload and torch.view_as_complex in the aten implementation right?

lezcano · 2023-09-02T22:20:57Z

torch/_meta_registrations.py

+        new_strides[-1] = 1
+
+        new_size = list(self.size())
+        new_size[self.dim() - 1] //= size_ratio


nit. In general, unless you know that you are sure that what you are working with are Python types, avoid in-place ops, as in-place ops and tracing often don't get along. I'm not sure if torch.SymInt handles in-place ops well, but better safe than sorry.

ezyang · 2023-09-03T23:50:47Z

#108005 (comment)

Yep

peterbell10 · 2023-09-04T13:02:50Z

view_as_real and view_as_complex already support SymInts but I think decomposing them to aten.view.dtype still makes sense as it's a far more general version of the same concept.

ezyang · 2023-09-07T13:08:23Z

@pytorchbot merge

pytorchmergebot · 2023-09-07T13:10:15Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

torch/_refs/__init__.py

aten/src/ATen/native/TensorConversions.cpp

pytorchmergebot · 2023-09-07T14:16:20Z

Merge failed

Reason: New commits were pushed while merging. Please rerun the merge command.

Details for Dev Infra team

Raised by workflow job

Fidget-Spinner · 2023-09-07T16:10:07Z

Peter, thanks for taking the time to review my PR thoroughly. Really appreciate it!

torch/_refs/__init__.py

ezyang · 2023-09-07T23:47:35Z

@pytorchbot merge

pytorchmergebot · 2023-09-07T23:49:14Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

Fidget-Spinner added 4 commits August 20, 2023 15:04

first try

06e20e8

move to ref, add tests

1cc5753

reduce diff

aa7def2

flake8

c1eae97

Fidget-Spinner requested review from mruberry and ngimel as code owners August 26, 2023 12:11

github-actions bot added module: inductor ciflow/inductor labels Aug 26, 2023

Fidget-Spinner commented Aug 26, 2023

View reviewed changes

pytorchbot added the open source label Aug 26, 2023

Fidget-Spinner added the topic: not user facing topic category label Aug 26, 2023

lint + ignore for torchinductor

2641069

peterbell10 requested changes Aug 26, 2023

View reviewed changes

Fidget-Spinner mentioned this pull request Aug 27, 2023

Meta registration for aten.view.dtype is wrong #108020

Closed

rename function

93855e8

bdhirsh added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 30, 2023

Fidget-Spinner added 2 commits September 2, 2023 21:49

rename

27fb55d

Fix meta registration for aten.view.dtype

cc3b999

Fidget-Spinner commented Sep 2, 2023

View reviewed changes

lint and partially attribute to peter for torch._set

fc899b4

Co-Authored-By: peterbell10 <13238737+peterbell10@users.noreply.github.com>

lezcano reviewed Sep 2, 2023

View reviewed changes

Fidget-Spinner added 2 commits September 7, 2023 20:48

Symbolify

c950d11

Merge remote-tracking branch 'upstream/main' into decomp_view_as_complex

b8e1a30

ezyang approved these changes Sep 7, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 7, 2023

pytorchmergebot added the merging label Sep 7, 2023

peterbell10 requested changes Sep 7, 2023

View reviewed changes

torch/_refs/__init__.py Outdated Show resolved Hide resolved

torch/_refs/__init__.py Outdated Show resolved Hide resolved

aten/src/ATen/native/TensorConversions.cpp Outdated Show resolved Hide resolved

aten/src/ATen/native/TensorConversions.cpp Outdated Show resolved Hide resolved

Address review

8560532

pytorchmergebot removed the merging label Sep 7, 2023

peterbell10 reviewed Sep 7, 2023

View reviewed changes

torch/_refs/__init__.py Outdated Show resolved Hide resolved

Remove extraneous check

93ce05d

peterbell10 approved these changes Sep 7, 2023

View reviewed changes

pytorchmergebot added the merging label Sep 7, 2023

pytorchmergebot added Merged and removed merging labels Sep 7, 2023

pytorchmergebot closed this in c458fa0 Sep 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Decompose/add reference for `view_as_complex` #108005

Decompose/add reference for `view_as_complex` #108005

Fidget-Spinner commented Aug 26, 2023 •

edited

pytorch-bot bot commented Aug 26, 2023 •

edited

Fidget-Spinner Aug 26, 2023

peterbell10 Aug 26, 2023

peterbell10 Aug 26, 2023

Fidget-Spinner Aug 27, 2023

peterbell10 Aug 26, 2023

Fidget-Spinner Aug 27, 2023

peterbell10 Aug 29, 2023

lezcano Sep 7, 2023

peterbell10 Sep 7, 2023

lezcano Sep 7, 2023

peterbell10 Sep 7, 2023

Fidget-Spinner Sep 2, 2023

ezyang commented Sep 2, 2023

Fidget-Spinner commented Sep 2, 2023

lezcano Sep 2, 2023

ezyang commented Sep 3, 2023

peterbell10 commented Sep 4, 2023

ezyang commented Sep 7, 2023

pytorchmergebot commented Sep 7, 2023

pytorchmergebot commented Sep 7, 2023

Fidget-Spinner commented Sep 7, 2023

ezyang commented Sep 7, 2023

pytorchmergebot commented Sep 7, 2023

	@register_meta(aten.view.dtype)
	def view_dtype(self, dtype):
	return utils.clone_preserve_strides(self).to(dtype)

		@@ -918,6 +919,47 @@ def trunc(a):
		return prims.trunc(a)


		# TODO: register this as a real ref/decomposition once TorchInductor supports complex!

		@@ -2606,7 +2606,54 @@ def meta_complex(real, imag):

		@register_meta(aten.view.dtype)
		def view_dtype(self, dtype):

Decompose/add reference for view_as_complex #108005

Decompose/add reference for view_as_complex #108005

Conversation

Fidget-Spinner commented Aug 26, 2023 • edited

pytorch-bot bot commented Aug 26, 2023 • edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/108005

✅ No Failures

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezyang commented Sep 2, 2023

Fidget-Spinner commented Sep 2, 2023

Choose a reason for hiding this comment

ezyang commented Sep 3, 2023

peterbell10 commented Sep 4, 2023

ezyang commented Sep 7, 2023

pytorchmergebot commented Sep 7, 2023

Merge started

pytorchmergebot commented Sep 7, 2023

Merge failed

Fidget-Spinner commented Sep 7, 2023

ezyang commented Sep 7, 2023

pytorchmergebot commented Sep 7, 2023

Merge started

Decompose/add reference for `view_as_complex` #108005

Decompose/add reference for `view_as_complex` #108005

Fidget-Spinner commented Aug 26, 2023 •

edited

pytorch-bot bot commented Aug 26, 2023 •

edited