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

fix pixel_shuffle return empty #6907

Merged
merged 3 commits into from
Apr 11, 2024
Merged

fix pixel_shuffle return empty #6907

merged 3 commits into from
Apr 11, 2024

Conversation

zpcore
Copy link
Collaborator

@zpcore zpcore commented Apr 10, 2024

Fixes #5886. Basically lowered the op using the XlaOps instead of going through pytorch decomposition.

Removed ExpectCounterChanged("xla::permute_copy", cpp_test::GetIgnoredCounters()); because we don't call xla::permute_copy in the lowering.

@zpcore zpcore requested a review from wonjoolee95 April 10, 2024 00:47
@zpcore zpcore marked this pull request as ready for review April 10, 2024 01:40
@zpcore zpcore requested a review from JackCaoG April 10, 2024 16:31
@JackCaoG
Copy link
Collaborator

Do you know if pixel_shuffle a view op?

@zpcore
Copy link
Collaborator Author

zpcore commented Apr 10, 2024

Do you know if pixel_shuffle a view op?

This is how pytorch do the composition: https://github.com/pytorch/pytorch/blob/58047205ed098c04ec045e66fc39dcc70b60600b/torch/_refs/nn/functional/__init__.py#L1183-L1190.

XlaOp:Reshape served the functionality of view op here. That's why we don't see view in our lowering.

@JackCaoG
Copy link
Collaborator

I guess my question is, if you do something like

input = torch.randn(1, 9, 4, 4)
output = torch.nn.functional.pixel_shuffle(input, 3)
input[0][0][0][0] = 1

I would expect output to also change since it is a view op. Do we still have that property with this lowering?

@zpcore
Copy link
Collaborator Author

zpcore commented Apr 10, 2024

No, it's not. Just tested on both CPU and XLA device, the input and output are in different shapes.

@JackCaoG
Copy link
Collaborator

ok.. yea input and output will be different shape, but if on CPU you modify the source tensor, output tensor won't get modified, it is a not a view op

@zpcore
Copy link
Collaborator Author

zpcore commented Apr 10, 2024

Oh I see, I should modify the contents to test the view. Thanks!

@zpcore zpcore merged commit 43f1071 into master Apr 11, 2024
18 checks passed
@zpcore zpcore deleted the piz/fix_pixel_shuffle branch April 11, 2024 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core ATen Opset] Lower aten_pixel_shuffle
3 participants