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

Jump to bottom

Add Half support for AvgPool2d on CPU #109578

Closed
wants to merge 1 commit into from
Closed

Add Half support for AvgPool2d on CPU #109578

wants to merge 1 commit into from
+38 −32

Conversation

CaoE
Copy link
Collaborator

@CaoE CaoE commented Sep 19, 2023 •
edited by pytorch-bot bot

Add Half support for AvgPool2d (both channels last and channels first) on CPU

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 19, 2023 •
edited

🔗 Helpful Links

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

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

⏳ No Failures, 1 Pending

As of commit 078a606 with merge base fe01605 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@github-actions github-actions bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Sep 19, 2023
@CaoE CaoE added module: half Related to float16 half-precision floats ciflow/trunk Trigger trunk jobs on your pull request ciflow/mps Run MPS tests (subset of trunk) ciflow/inductor labels Sep 19, 2023
@CaoE CaoE changed the title Add Half support for AvgPool2d (both channels last and channels first) on CPU Add Half support for AvgPool2d on CPU Sep 19, 2023
@CaoE CaoE added the topic: not user facing topic category label Sep 21, 2023
@CaoE CaoE force-pushed the fp16_avgpool branch 2 times, most recently from f28b0e5 to c1f1f3a Compare September 26, 2023 07:12
@CaoE CaoE requested a review from mingfeima October 16, 2023 06:37
@CaoE CaoE force-pushed the fp16_avgpool branch 3 times, most recently from b8f3bd3 to 39f5e2b Compare October 18, 2023 06:05
@CaoE
Copy link
Collaborator Author

CaoE commented Oct 18, 2023

Cherry-picked from #96080.

@CaoE CaoE force-pushed the fp16_avgpool branch 2 times, most recently from 3abe1c7 to 31d4c06 Compare October 19, 2023 02:11
@CaoE CaoE requested a review from jgong5 October 24, 2023 00:45
@CaoE CaoE added release notes: nn release notes category and removed topic: not user facing topic category labels Nov 20, 2023
@mingfeima mingfeima marked this pull request as ready for review November 21, 2023 01:35
@CaoE CaoE requested a review from cpuhrsch November 21, 2023 01:37
@CaoE
Copy link
Collaborator Author

CaoE commented Nov 21, 2023

@cpuhrsch Could you please review this PR ? Thank you.

@CaoE
Copy link
Collaborator Author

CaoE commented Dec 7, 2023

@cpuhrsch Could you please review this PR ? Thank you.

cpuhrsch
cpuhrsch reviewed Dec 8, 2023
View reviewed changes
test/test_mps.py Outdated
@@ -799,7 +799,7 @@ def mps_ops_modifier(ops):
'logit': [torch.float16],
}

EMPTY_OPS_SKIPLIST = {
OPS_SKIPLIST = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you keep the old EMPTY_OPS_SKIPLIST and create a new dictionary for this? Just in case there's more assumptions baked into EMPTY_OPS_SKIPLIST. Also cc @albanD for the right PoC since this is an MPS related change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes put it in the skiplist below please.
This one is related to non-determinisms of the empty op only

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your comments. Fixed.

cpuhrsch
cpuhrsch reviewed Dec 8, 2023
View reviewed changes
Copy link
Contributor

@cpuhrsch cpuhrsch left a comment

Choose a reason for hiding this comment

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

Changes not related to MPS look fine.

albanD
albanD approved these changes Dec 11, 2023
View reviewed changes
Copy link
Collaborator

@albanD albanD left a comment •
edited

Choose a reason for hiding this comment

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

Thanks!

FYI @kulinseth for the MPS skip

@CaoE
Copy link
Collaborator Author

CaoE commented Dec 12, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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

guilhermeleobas pushed a commit to guilhermeleobas/pytorch that referenced this pull request Dec 18, 2023
@mingfeima @guilhermeleobas
Add Half support for AvgPool2d on CPU (pytorch#109578)
0fd5a86 
Add Half support for AvgPool2d (both channels last and channels first) on CPU

Pull Request resolved: pytorch#109578
Approved by: https://github.com/mingfeima, https://github.com/albanD
hyperfraise pushed a commit to hyperfraise/pytorch that referenced this pull request Dec 21, 2023
@mingfeima @hyperfraise
Add Half support for AvgPool2d on CPU (pytorch#109578)
96c3ac7 
Add Half support for AvgPool2d (both channels last and channels first) on CPU

Pull Request resolved: pytorch#109578
Approved by: https://github.com/mingfeima, https://github.com/albanD
hyperfraise pushed a commit to hyperfraise/pytorch that referenced this pull request Dec 21, 2023
@mingfeima @hyperfraise
Add Half support for AvgPool2d on CPU (pytorch#109578)
bc2e622 
Add Half support for AvgPool2d (both channels last and channels first) on CPU

Pull Request resolved: pytorch#109578
Approved by: https://github.com/mingfeima, https://github.com/albanD
ZhiweiYan-96 pushed a commit to ZhiweiYan-96/pytorch that referenced this pull request Dec 22, 2023
@mingfeima @ZhiweiYan-96
Add Half support for AvgPool2d on CPU (pytorch#109578)
1ba14e7 
Add Half support for AvgPool2d (both channels last and channels first) on CPU

Pull Request resolved: pytorch#109578
Approved by: https://github.com/mingfeima, https://github.com/albanD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpuhrsch cpuhrsch cpuhrsch left review comments

@albanD albanD albanD approved these changes

@mingfeima mingfeima mingfeima approved these changes

@jgong5 jgong5 Awaiting requested review from jgong5

@kulinseth kulinseth Awaiting requested review from kulinseth kulinseth is a code owner

@mruberry mruberry Awaiting requested review from mruberry mruberry is a code owner

@ngimel ngimel Awaiting requested review from ngimel

Assignees
No one assigned
Labels
ciflow/inductor ciflow/mps Run MPS tests (subset of trunk) ciflow/trunk Trigger trunk jobs on your pull request Merged module: cpu CPU specific problem (e.g., perf, algorithm) module: half Related to float16 half-precision floats open source release notes: nn release notes category
Projects
PyTorch Intel
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@CaoE @pytorchmergebot @cpuhrsch @albanD @mingfeima @pytorchbot