Support MPS Backend also on iOS < 16 (#9089)#9095
Support MPS Backend also on iOS < 16 (#9089)#9095facebook-github-bot merged 1 commit intopytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9095
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 5421a77 with merge base 4c54bab ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@pytorchbot label "topic: not user facing" |
|
This pull request was exported from Phabricator. Differential Revision: D70795041 |
|
This pull request was exported from Phabricator. Differential Revision: D70795041 |
cd9846d to
53a893c
Compare
|
@DenisVieriu97 please take a look! |
|
@DenisVieriu97 shall we just merge it? |
| Debug, "%s %d: %d", | ||
| __FUNCTION__, graphNode->input1_id(), graphNode->output_id() | ||
| ); | ||
| if (@available(iOS 16, macOS 13, *)) { |
There was a problem hiding this comment.
Should we return an error for the else statement
There was a problem hiding this comment.
If the if is not true, then we do not do _idToMPSGraphTensor[graphNode->output_id()] = ... which I think would then not set the tensor value and that would fail the prediction, but only if this is used.
Do you think we should return Error::notSupported or something like that?
There was a problem hiding this comment.
Yeah, just we can handle it in a non-fatal way.
There was a problem hiding this comment.
@cccclai Is the latest version OK?
Thank you for the review :)
53a893c to
c09c4ac
Compare
Summary: Differential Revision: D70795041
c09c4ac to
675934a
Compare
Summary: Differential Revision: D70795041
|
This pull request was exported from Phabricator. Differential Revision: D70795041 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D70795041 |
Summary: Pull Request resolved: pytorch#9095 Pull Request resolved: pytorch#9089 Differential Revision: D70795041
675934a to
09267c4
Compare
|
|
||
| return Error::Ok; | ||
| } else { | ||
| return Error::NotSupported; |
There was a problem hiding this comment.
Thanks, one minor thing - can you add an error log here like
ET_LOG(Error, "...")
There was a problem hiding this comment.
Added, and I also changed the requirements to iOS 15.4 and MacOS 12.3
Summary: Reviewed By: cccclai Differential Revision: D70795041
09267c4 to
02d0646
Compare
Summary: Reviewed By: cccclai Differential Revision: D70795041
02d0646 to
c1b1109
Compare
|
This pull request was exported from Phabricator. Differential Revision: D70795041 |
Summary: Pull Request resolved: pytorch#9095 Pull Request resolved: pytorch#9089 Reviewed By: cccclai Differential Revision: D70795041
|
This pull request was exported from Phabricator. Differential Revision: D70795041 |
c1b1109 to
5421a77
Compare
DenisVieriu97
left a comment
There was a problem hiding this comment.
Looks good - thanks @f-meloni for the changes
| #if defined(__MAC_13_0) | ||
| if (macOS13Plus) { | ||
| languageVersion = MTLLanguageVersion3_0; | ||
| if (@available(iOS 16, macOS 13, *)) { |
There was a problem hiding this comment.
Thank you for the change
| Debug, "%s %d: %d", | ||
| __FUNCTION__, graphNode->input1_id(), graphNode->output_id() | ||
| ); | ||
| if (@available(iOS 15.4, macOS 12.3, *)) { |
Differential Revision: D70795041 Pull Request resolved: pytorch#9095
Differential Revision: D70795041 Pull Request resolved: pytorch#9095
Summary: Pull Request resolved: #9089
Differential Revision: D70795041
The MPS backend currently does not compile for iOS < 16.
I have added some conditional code to make it usable anyway