Skip to content

Use NNPACK for strided convolutions. #29595

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

Use NNPACK for strided convolutions. #29595

AshkanAliabadi wants to merge 1 commit into from

Conversation

@AshkanAliabadi
Copy link
Contributor

@AshkanAliabadi AshkanAliabadi commented Nov 11, 2019

Use NNPACK for strided convolutions.

ResNet50 on Pixel 3:

  • Before: 552.956 ms
  • After: 402.947 ms

houseroad
houseroad reviewed Nov 12, 2019
View reviewed changes
Copy link
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

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

New function should be backward compatible. Could you fix the CI and trigger the backward_compatible_test?

facebook-github-bot
facebook-github-bot reviewed Nov 12, 2019
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.

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

@ljk53
Copy link
Contributor

ljk53 commented Nov 12, 2019

New function should be backward compatible. Could you fix the CI and trigger the backward_compatible_test?

Good suggestion. We should run the test to confirm. For this specific PR it only adds an internal op (starting with "_", not exposed to python) so it shouldn't break any external contract if we have done it correctly :)

facebook-github-bot
facebook-github-bot reviewed Nov 12, 2019
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.

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

@AshkanAliabadi
Copy link
Contributor Author

AshkanAliabadi commented Nov 13, 2019

Synced offline with Lu and ran the test locally. That aside, there seems to be something wrong with the CI.

@houseroad houseroad mentioned this pull request Nov 13, 2019
Closed
@houseroad
Copy link
Member

houseroad commented Nov 13, 2019

The whole CI is broken at this moment as well.

facebook-github-bot pushed a commit that referenced this pull request Nov 13, 2019
@houseroad @facebook-github-bot
Allow unrelated histories when rebasing to master (#29699)
03d021d 
Summary:
Some prs are refused to be merged.

For example, #29595
Pull Request resolved: #29699

Reviewed By: hl475

Differential Revision: D18473531

Pulled By: houseroad

fbshipit-source-id: e7a4eb1b4be9d9da6dc281575eeb4d7ae685b531
facebook-github-bot
facebook-github-bot reviewed Nov 13, 2019
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.

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

zdevito pushed a commit to zdevito/ATen that referenced this pull request Nov 14, 2019
@facebook-github-bot
Use NNPACK for strided convolutions. (#29595)
994e86f 
Summary:
Use NNPACK for strided convolutions.

ResNet50 on Pixel 3:
- Before: 552.956 ms
- After: 402.947 ms
Pull Request resolved: pytorch/pytorch#29595

Reviewed By: houseroad

Differential Revision: D18457472

Pulled By: AshkanAliabadi

fbshipit-source-id: 51f22ce120c39f197cd564bcc71bbad2951edf85
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Nov 14, 2019

@AshkanAliabadi merged this pull request in 9ee6fa0.

csarofeen pushed a commit to mruberry/pytorch that referenced this pull request Nov 18, 2019
@csarofeen
Use NNPACK for strided convolutions. (pytorch#29595)
1f7c09f 
Summary:
Use NNPACK for strided convolutions.

ResNet50 on Pixel 3:
- Before: 552.956 ms
- After: 402.947 ms
Pull Request resolved: pytorch#29595

Reviewed By: houseroad

Differential Revision: D18457472

Pulled By: AshkanAliabadi

fbshipit-source-id: 51f22ce120c39f197cd564bcc71bbad2951edf85
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

@houseroad houseroad houseroad left review comments

@ljk53 ljk53 Awaiting requested review from ljk53

Assignees

No one assigned

Labels

Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AshkanAliabadi @ljk53 @houseroad @facebook-github-bot @mruberry