Skip to content

Check if input is ChannelsLast or ChannelsLast3d for quantized AdaptivePool3d. #42780

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
thinking-tower wants to merge 1 commit into from
Closed

Check if input is ChannelsLast or ChannelsLast3d for quantized AdaptivePool3d. #42780

thinking-tower wants to merge 1 commit into from

Conversation

@thinking-tower
Copy link
Contributor

@thinking-tower thinking-tower commented Aug 8, 2020 •
edited
Loading

cc @z-a-f, @vkuzo. This serves as a very simple first step to the issue mentioned in #42779.

Description

Since ChannelsLast and ChannelsLast3d are not equivalent (MemoryFormat.h), the "fast" path for NDHWC is ignored.

This PR would produce the expected behaviour for 4 (5 if including batch) dimensional tensors.

Benchmarks

Notes

  • For all when channels < 8, it is actually slower than before.
  • For qint32, it is actually 2x slower than before.
  • For qint8 and quint8 when 8 < channels < 64, the execution time decreases up to 9-10 times in the benchmarks.
  • While execution time does improve, it remains slower than the contiguous variant when channels > 64.

C++

before_after_py

Python

before_after_cpp

Reproduce

See #42779.

@dr-ci
Copy link

dr-ci bot commented Aug 8, 2020 •
edited
Loading

💊 CI failures summary and remediations

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

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

🕵️ 2 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build binary_linux_libtorch_3_7m_cpu_devtoolset7_shared-with-deps_build (1/2)

Step: "Checkout pytorch/builder repo" (full log | diagnosis details | 🔁 rerun)

fatal: reference is not a tree: f015d698006c4a11be15b1ebb75b3b9bb317b914 
fatal: reference is not a tree: f015d698006c4a11be15b1ebb75b3b9bb317b914 
Unable to checkout 'f015d698006c4a11be15b1ebb75b3b9bb317b914' in submodule path 'third_party/tensorpipe' 
+ sleep 4 
+ git submodule update --init --recursive 
Warning: Permanently added the RSA host key for IP address '140.82.114.4' to the list of known hosts.  
fatal: reference is not a tree: f015d698006c4a11be15b1ebb75b3b9bb317b914 
Unable to checkout 'f015d698006c4a11be15b1ebb75b3b9bb317b914' in submodule path 'third_party/tensorpipe' 
+ sleep 8 
+ git submodule update --init --recursive 
Warning: Permanently added the RSA host key for IP address '140.82.112.4' to the list of known hosts.  
fatal: reference is not a tree: f015d698006c4a11be15b1ebb75b3b9bb317b914 
Unable to checkout 'f015d698006c4a11be15b1ebb75b3b9bb317b914' in submodule path 'third_party/tensorpipe'

See CircleCI build caffe2_onnx_main_py3_6_clang7_ubuntu16_04_build (2/2)

Step: "Build" (full log | diagnosis details | 🔁 rerun)

Aug 08 14:25:12 fatal: reference is not a tree: f015d698006c4a11be15b1ebb75b3b9bb317b914 
DOCKER_IMAGE: 308535385114.dkr.ecr.us-east-1.amazonaws.com/caffe2/py3.6-clang7-ubuntu16.04:376 
 
real	0m29.408s 
user	0m0.073s 
sys	0m0.069s 
Aug 08 14:24:10 ++ export BUILD_ENVIRONMENT=caffe2-onnx-main-py3.6-clang7-ubuntu16.04-build 
Aug 08 14:24:10 ++ BUILD_ENVIRONMENT=caffe2-onnx-main-py3.6-clang7-ubuntu16.04-build 
Aug 08 14:24:10 ++ git submodule sync 
Aug 08 14:24:10 ++ git submodule update -q --init --recursive 
Aug 08 14:25:12 fatal: reference is not a tree: f015d698006c4a11be15b1ebb75b3b9bb317b914 
Aug 08 14:25:15 Unable to checkout 'f015d698006c4a11be15b1ebb75b3b9bb317b914' in submodule path 'third_party/tensorpipe'

ci.pytorch.org: 2 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 1 time.

@thinking-tower thinking-tower changed the title Check if input is ChannelsLast or ChannelsLast3d. Check if input is ChannelsLast or ChannelsLast3d for quantized AdaptivePool3d. Aug 9, 2020
z-a-f
z-a-f approved these changes Aug 10, 2020
View reviewed changes
Copy link

@z-a-f z-a-f left a comment

Choose a reason for hiding this comment

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

This is a great catch! Thank you. cc'ing @jerryzh168 for an extra pair of eyes to take a look

facebook-github-bot
facebook-github-bot reviewed Aug 10, 2020
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@z-a-f has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@z-a-f z-a-f requested a review from jerryzh168 August 10, 2020 19:25
@thinking-tower
Copy link
Contributor Author

thinking-tower commented Aug 13, 2020

Hey @z-a-f @jerryzh168, what's the state of this PR?

@jerryzh168
Copy link
Contributor

jerryzh168 commented Aug 13, 2020

It's imported to phabricator, @z-a-f will need to land this

@thinking-tower thinking-tower requested a review from z-a-f August 26, 2020 09:54
@z-a-f
Copy link

z-a-f commented Aug 26, 2020

Rebasing it now -- once it completes, will land it!

@thinking-tower
Copy link
Contributor Author

thinking-tower commented Aug 27, 2020

@z-a-f Awesome!

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 27, 2020

@z-a-f merged this pull request in 5da97a3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@jerryzh168 jerryzh168 Awaiting requested review from jerryzh168

@z-a-f z-a-f Awaiting requested review from z-a-f

Assignees

No one assigned

Labels

Merged open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@thinking-tower @jerryzh168 @z-a-f @facebook-github-bot @pytorchbot @mruberry