Skip to content

[MPS] Add MPS implementation for constant_pad_nd() (#75) #82366

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

[MPS] Add MPS implementation for constant_pad_nd() (#75) #82366

kulinseth wants to merge 1 commit into from

Conversation

@kulinseth
Copy link
Collaborator

@kulinseth kulinseth commented Jul 27, 2022

MPS has a native implementation of the constant pad nd. Adding that instead of going through the view ops helps improve performance in several benchmarks in torchbench.

@kulinseth kulinseth requested review from malfet and razarmehr July 27, 2022 21:01
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 27, 2022 •
edited
Loading

🔗 Helpful links

❌ 1 New Failures, 1 Base Failures

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

Expand to see more
  • 1/2 failures introduced in this PR
  • 1/2 broken upstream at merge base 2bfae07 on Jul 28 from 3:34pm to 6:01pm

🕵️ 1 new failure recognized by patterns

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

See GitHub Actions build trunk / ios-12-5-1-x86-64 / build (1/1)

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

2022-07-29T02:39:36.0716740Z ##[error]The operation was canceled. 
2022-07-29T02:39:16.3445790Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/1x4c2s4-minmax-fp32-sse41-ld128.c.o
2022-07-29T02:39:17.8676450Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/1x4c8-minmax-fp32-sse41-ld64.c.o
2022-07-29T02:39:17.8783530Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/1x4c8-minmax-fp32-sse41-ld128.c.o
2022-07-29T02:39:21.0239530Z [ 51%] Building CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/Operators_3.cpp.o
2022-07-29T02:39:21.2389890Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/2x4c2-minmax-fp32-sse41-ld64.c.o
2022-07-29T02:39:24.0215120Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/2x4c2-minmax-fp32-sse41-ld128.c.o
2022-07-29T02:39:26.1269410Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/2x4c2s4-minmax-fp32-sse41-ld64.c.o
2022-07-29T02:39:27.6151590Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/2x4c2s4-minmax-fp32-sse41-ld128.c.o
2022-07-29T02:39:29.7701180Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/2x4c8-minmax-fp32-sse41-ld64.c.o
2022-07-29T02:39:33.1411820Z [ 51%] Building C object confu-deps/XNNPACK/CMakeFiles/all_microkernels.dir/src/qc8-igemm/gen/2x4c8-minmax-fp32-sse41-ld128.c.o
2022-07-29T02:39:36.0716740Z ##[error]The operation was canceled.
2022-07-29T02:39:36.2831890Z Post job cleanup.
2022-07-29T02:39:36.5071290Z Post job cleanup.
2022-07-29T02:39:41.6328030Z [command]/usr/local/bin/git version
2022-07-29T02:39:41.8216350Z git version 2.37.1
2022-07-29T02:39:41.9894720Z Copying '/Users/runner/.gitconfig' to '/Users/runner/work/_temp/7e256095-a8c3-4033-bdfc-46467d55a9b8/.gitconfig'
2022-07-29T02:39:42.0010290Z Temporarily overriding HOME='/Users/runner/work/_temp/7e256095-a8c3-4033-bdfc-46467d55a9b8' before making global git config changes
2022-07-29T02:39:42.5513950Z Adding repository directory to the temporary git global config as a safe directory
2022-07-29T02:39:42.5667330Z [command]/usr/local/bin/git config --global --add safe.directory /Users/runner/work/pytorch/pytorch
2022-07-29T02:39:42.5687620Z [command]/usr/local/bin/git config --local --name-only --get-regexp core\.sshCommand
2022-07-29T02:39:42.5973940Z [command]/usr/local/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :

🚧 1 fixed upstream failure:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

If your commit is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD
This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@kulinseth kulinseth added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 27, 2022
razarmehr
razarmehr approved these changes Jul 28, 2022
View reviewed changes
Copy link
Collaborator

@razarmehr razarmehr left a comment

Choose a reason for hiding this comment

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

Looks good.

@kulinseth
Copy link
Collaborator Author

kulinseth commented Jul 28, 2022 

NotImplementedError: Could not run 'aten::transpose.int' with arguments from the 'NestedTensorCPU' backend. This could be because the operator doesn't exist for this backend, or was omitted during the selective/custom build process (if using custom build). If you are a Facebook employee using PyTorch on mobile, please visit https://fburl.com/ptmfixes for possible resolutions. 'aten::transpose.int' is only available for these backends: [CPU, CUDA, HIP, XLA, MPS, IPU, XPU, HPU, VE, Lazy, Meta, PrivateUse1, PrivateUse2, PrivateUse3, FPGA, ORT, Vulkan, Metal, QuantizedCPU, QuantizedCUDA, QuantizedHIP, QuantizedXLA, QuantizedMPS, QuantizedIPU, QuantizedXPU, QuantizedHPU, QuantizedVE, QuantizedLazy, QuantizedMeta, QuantizedPrivateUse1, QuantizedPrivateUse2, QuantizedPrivateUse3, CustomRNGKeyId, MkldnnCPU, SparseCPU, SparseCUDA, SparseHIP, SparseXLA, SparseMPS, SparseIPU, SparseXPU, SparseHPU, SparseVE, SparseLazy, SparseMeta, SparsePrivateUse1, SparsePrivateUse2, SparsePrivateUse3, SparseCsrCPU, SparseCsrCUDA, BackendSelect, Python, Functionalize, Named, Conjugate, Negative, ZeroTensor

There seems to be a NestedTensor regression.

@kulinseth
Copy link
Collaborator Author

kulinseth commented Jul 28, 2022

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Jul 28, 2022

@pytorchbot successfully started a rebase job. Check the current status here

@razarmehr @pytorchmergebot
Add MPS implementation for constant_pad_nd() (#75)
0b1fdc3 
This should improve the performance in several benchmarks
@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Jul 28, 2022

Successfully rebased constant_pad_nd onto refs/remotes/origin/master, please pull locally before adding more changes (for example, via git checkout constant_pad_nd && git pull --rebase)

@kulinseth
Copy link
Collaborator Author

kulinseth commented Jul 29, 2022

@pytorchbot merge -f "Unrelated issue."

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Jul 29, 2022

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Contributor

github-actions bot commented Jul 29, 2022

Hey @kulinseth.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

facebook-github-bot pushed a commit that referenced this pull request Aug 1, 2022
@razarmehr @facebook-github-bot
[MPS] Add MPS implementation for constant_pad_nd() (#75) (#82366) (#8…
b8192c2 
…2366)

Summary:
MPS has a native implementation of the constant pad nd. Adding that instead of going through the view ops helps improve performance in several benchmarks in torchbench.

Pull Request resolved: #82366
Approved by: https://github.com/malfet, https://github.com/razarmehr

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/38b4114278bcab35f4221945b2777c8c1fff37a0

Reviewed By: osalpekar

Differential Revision: D38306822

Pulled By: osalpekar

fbshipit-source-id: 36880d4d638fd85bd9ed987c11103fdf1fc48279
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malfet malfet malfet approved these changes

@razarmehr razarmehr razarmehr approved these changes

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request cla signed Merged open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@kulinseth @facebook-github-bot @pytorchmergebot @malfet @razarmehr @pytorchbot