Skip to content

[quant] Quantized adaptive_avg_pool3d #40271

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.

Sign up for GitHub

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

Jump to bottom
Closed
z-a-f wants to merge 7 commits into from
Closed

[quant] Quantized adaptive_avg_pool3d #40271

z-a-f wants to merge 7 commits into from

Conversation

@z-a-f
Copy link

@z-a-f z-a-f commented Jun 19, 2020 •
edited
Loading

Stack from ghstack:

Closes #40244

Differential Revision: D22134318

@z-a-f z-a-f requested a review from apaszke as a code owner June 19, 2020 05:52
@z-a-f z-a-f requested review from jerryzh168 and vkuzo June 19, 2020 05:55
z-a-f pushed a commit that referenced this pull request Jun 19, 2020
[quant] Quantized adaptive_avg_pool3d
de47a28 
Closes #40244

ghstack-source-id: c51fa1f
Pull Request resolved: #40271
@dr-ci
Copy link

dr-ci bot commented Jun 19, 2020 •
edited
Loading

💊 CI failures summary and remediations

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

  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

ci.pytorch.org: 1 failed

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 18 times.

jerryzh168
jerryzh168 reviewed Jun 19, 2020
View reviewed changes
test/quantization/test_quantized_op.py Outdated
output_size_d=st.integers(1, 10),
output_size_h=st.integers(1, 10),
output_size_w=st.integers(1, 10))
def test_adaptive_avg_pool3d(self, X, output_size_d, output_size_h,
Copy link
Contributor

@jerryzh168 jerryzh168 Jun 19, 2020

Choose a reason for hiding this comment

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

there are many code duplications in these test, would be great if we can dedup some, I think dedup this one with ndhwc would be striaghtforward? also dedup with the 2d test as well if possible(maybe part of the code).

Copy link
Author

@z-a-f z-a-f Jun 23, 2020

Choose a reason for hiding this comment

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

I partially refactored to remove some dups, but the ndhwc is much harder -- will refactor later

jerryzh168
jerryzh168 reviewed Jun 19, 2020
View reviewed changes
jerryzh168
jerryzh168 reviewed Jun 19, 2020
View reviewed changes
aten/src/ATen/native/native_functions.yaml
dispatch:
CPU: adaptive_avg_pool3d_cpu
CUDA: adaptive_avg_pool3d_cuda
QuantizedCPU: quantized_adaptive_avg_pool3d
Copy link
Contributor

@jerryzh168 jerryzh168 Jun 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I feel we should rename this, it should be adaptive_avg_pool3d_quantized_cpu same for others, we need to refactor other names as well before we can expand to other backends

Copy link
Author

@z-a-f z-a-f Jun 19, 2020

Choose a reason for hiding this comment

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

I think I can do it in a separate PR -- it involves changing other ones. WIll creaete a task for myself

Copy link
Contributor

@jerryzh168 jerryzh168 Jun 19, 2020

Choose a reason for hiding this comment

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

yeah we don't need to it in this PR, sounds good, thanks

Copy link
Author

@z-a-f z-a-f Jun 19, 2020

Choose a reason for hiding this comment

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

#40315

Copy link
Author

@z-a-f z-a-f Jul 10, 2020

Choose a reason for hiding this comment

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

#41071

jerryzh168
jerryzh168 reviewed Jun 19, 2020
View reviewed changes
@z-a-f z-a-f mentioned this pull request Jun 19, 2020
Open
jerryzh168
jerryzh168 approved these changes Jun 22, 2020
View reviewed changes
Copy link
Contributor

@jerryzh168 jerryzh168 left a comment

Choose a reason for hiding this comment

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

Please fix CI errors before land

z-a-f pushed a commit that referenced this pull request Jun 23, 2020
[quant] Quantized adaptive_avg_pool3d
8e2d43f 
Closes #40244

ghstack-source-id: 9473f27
Pull Request resolved: #40271
z-a-f pushed a commit that referenced this pull request Jun 23, 2020
[quant] Quantized adaptive_avg_pool3d
cbb9d94 
Closes #40244

ghstack-source-id: c6549bf
Pull Request resolved: #40271
z-a-f pushed a commit that referenced this pull request Jun 24, 2020
[quant] Quantized adaptive_avg_pool3d
d5a21a6 
Closes #40244

ghstack-source-id: 6cfbc39
Pull Request resolved: #40271
z-a-f pushed a commit to z-a-f/pytorch that referenced this pull request Jun 24, 2020
[quant] Quantized adaptive_avg_pool3d (pytorch#40271)
1a8fe12 
Summary:
Pull Request resolved: pytorch#40271

Closes pytorch#40244

Test Plan: Imported from OSS

Reviewed By: vkuzo

Differential Revision: D22134318

Pulled By: z-a-f

fbshipit-source-id: 0489b6c083a3cbc21a1d81d8bfcc499372308088
@z-a-f z-a-f mentioned this pull request Jun 24, 2020
Closed
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 24, 2020

@z-a-f merged this pull request in c314e0d.

@facebook-github-bot facebook-github-bot deleted the gh/z-a-f/38/head branch June 27, 2020 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vkuzo vkuzo vkuzo left review comments

@jerryzh168 jerryzh168 jerryzh168 approved these changes

@apaszke apaszke Awaiting requested review from apaszke

Assignees

No one assigned

Labels

Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@z-a-f @facebook-github-bot @vkuzo @jerryzh168 @mruberry