-
Notifications
You must be signed in to change notification settings - Fork 26.3k
grid_sampler_3d for MPS #159421
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
grid_sampler_3d for MPS #159421
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/159421
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 2 Unrelated FailuresAs of commit 36d9498 with merge base c665594 ( NEW FAILURES - The following jobs have failed:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Attention! native_functions.yaml was changedIf you are adding a new function or defaulted argument to native_functions.yaml, you cannot use it from pre-existing Python frontend code until our FC window passes (two weeks). Split your PR into two PRs, one which adds the new C++ functionality, and one that makes use of it from Python, and land them two weeks apart. See https://github.com/pytorch/pytorch/wiki/PyTorch's-Python-Frontend-Backward-and-Forward-Compatibility-Policy#forwards-compatibility-fc for more info. Caused by: |
|
Any updates on this? @malfet |
|
I see that there was another PR #160541 that was opened after this one, but already merged. It looks like that the merged PR does not implement the backward pass or nearest interpolation. Therefore, my PR might still be worth merging. @malfet @kurtamohler |
|
Hey @mlaves, I'm sorry I had not noticed that you opened this PR. I have started to work on the backward part as well, but I could stop if you'd like to update your PR to only add the backward function. You'll need to enable tests for it by removing this line: pytorch/torch/testing/_internal/common_mps.py Line 665 in 06c7516
You can run the tests on your machine with @malfet , does that sound good to you? |
|
@mlaves sorry I've missed your change earlier (I usually go over all the changes that claim to Fix some of the MPS issues or has Please rebase your change and I'll have a look at it soon |
|
@kurtamohler No worries! I also missed your PR and saw it just now. It's rather remarkable that this has been missing for years, only to be implemented by two individuals within a couple of days lol. @malfet I will rebase my changes on current main as soon as possible. |
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
Metal implementation of
grid_sampler_3d, including backward pass.Runtime benchmarks vs. CPU (M2 Max)
Output accuracy test vs. CPU
Click to expand
Gradient accuracy test vs. CPU
Click to expand