[GPU] Enable Metal on macosx #47635

xta0 · 2020-11-09T23:27:07Z

Stack from ghstack:

[Person Seg] Compress the person seg model #48008 [Person Seg] Compress the person seg model
[GPU] Enable Metal on macosx #47635 [GPU] Enable Metal on macosx

Add macosx support for metal. The supported os version is 10.13 and above.

Differential Revision: D24825088

NOTE FOR REVIEWERS: This PR has internal Facebook specific changes or comments, please review them on Phabricator!

Add macosx support for metal. The supported os version is 10.13 and above. Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)! [ghstack-poisoned]

Add macosx support for metal. The supported os version is 10.13 and above. Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)! ghstack-source-id: 116242312 Pull Request resolved: #47635

dr-ci · 2020-11-10T00:58:38Z

💊 CI failures summary and remediations

As of commit 5a06789 (more details on the Dr. CI page):

2/8 failures possibly* introduced in this PR
- 2/2 non-CircleCI failure(s)
6/8 broken upstream at merge base fa0acb7 since Nov 16

🚧 6 ongoing upstream failures:

These were probably caused by upstream breakages that are not fixed yet:

pytorch_linux_backward_compatibility_check_test since Nov 16
- 🔁 rerun
pytorch_windows_vs2019_py36_cuda10.1_test2 since Nov 16
- 🔁 rerun
pytorch_linux_xenial_cuda10_2_cudnn7_py3_jit_legacy_test since Nov 16
- 🔁 rerun
pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test2 since Nov 16
- 🔁 rerun
pytorch_windows_vs2019_py36_cuda10.1_test1 since Nov 16
- 🔁 rerun
pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test1 since Nov 16
- 🔁 rerun

ci.pytorch.org: 2 failed

Failed: pr/pytorch-linux-bionic-rocm3.9-py3.6
Failed: pr/caffe2-pytorch-linux-bionic-rocm3.9-py3.6-test

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 28 times.

Add macosx support for metal. The supported os version is 10.13 and above. Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)! [ghstack-poisoned]

Pull Request resolved: #47635 Add macosx support for metal. The supported os version is 10.13 and above. ghstack-source-id: 116617355 Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)!

ljk53

LGTM, just some nits and questions about convention.

ljk53 · 2020-11-13T20:03:51Z

cmake/Metal.cmake

+if(NOT DEFINED CMAKE_OSX_DEVELOPER_ROOT)
+    if(EXISTS ${XCODE_POST_43_ROOT})
+        set(CMAKE_OSX_DEVELOPER_ROOT ${XCODE_POST_43_ROOT})
+    elseif(EXISTS ${XCODE_PRE_43_ROOT})
+        set(CMAKE_OSX_DEVELOPER_ROOT ${XCODE_PRE_43_ROOT})
+    endif(EXISTS ${XCODE_POST_43_ROOT})
+endif(NOT DEFINED CMAKE_OSX_DEVELOPER_ROOT)
+set(CMAKE_OSX_DEVELOPER_ROOT ${CMAKE_OSX_DEVELOPER_ROOT} CACHE PATH "Location of OSX SDKs root directory")
+
+if(NOT DEFINED CMAKE_OSX_SDK_ROOT)
+    file(GLOB _CMAKE_OSX_SDKS "${CMAKE_OSX_DEVELOPER_ROOT}/SDKs/*")
+    if(_CMAKE_OSX_SDKS) 
+        list(SORT _CMAKE_OSX_SDKS)
+        list(REVERSE _CMAKE_OSX_SDKS)
+        list(GET _CMAKE_OSX_SDKS 0 CMAKE_OSX_SDK_ROOT)
+        message(STATUS "_CMAKE_OSX_SDKS: ${_CMAKE_OSX_SDKS}")
+    else(_CMAKE_OSX_SDKS)
+        message(FATAL_ERROR "No OSX SDK's found in default search path ${CMAKE_OSX_DEVELOPER_ROOT}.")
+    endif(_CMAKE_OSX_SDKS)
+    message(STATUS "Toolchain using default OSX SDK: ${CMAKE_OSX_SDK_ROOT}")
+endif(NOT DEFINED CMAKE_OSX_SDK_ROOT)


Seems very similar to iOS.cmake - any chance to factor out?
also cc: @malfet @xuhdev

ljk53 · 2020-11-13T20:20:32Z

aten/src/ATen/native/metal/MetalPrepackOpRegister.cpp

@@ -3,7 +3,7 @@
 #include <ATen/ATen.h>


-#if defined(C10_IOS)
+#if (C10_IOS || TARGET_OS_MAC)


Nothing wrong about this - just feel that having C10_IOS (defined by us) and TARGET_OS_MAC (defined by apple) on the same condition is a little unbalanced, lol.

If this condition is going to be used at other places, shall we create a dedicated macro? Not sure what's the best convention in PyTorch codebase (cc: @dzhulgakov):

if it's targeting apple devices, shall we introduce 'C10_APPLE'?

or, can we simply use some master feature switch? e.g. '#if AT_METAL_ENABLED()' seems to be one of the common pattern.

How about (TARGET_OS_PHONE || TARGET_OS_MAC). Would that make you feel better 🤣

ljk53 · 2020-11-13T20:21:43Z

aten/src/ATen/native/metal/MetalPrepackOpRegister.cpp

  return mpscnn::conv2d(input, *op_context);
 #else
-  TORCH_CHECK(false, "conv2d_prepack_run can only be invoked on iOS");
+  TORCH_CHECK(false, "conv2d_prepack_run can only be invoked on iOS and MacOS");
  return input;


nit: I don't think we need 'return input;' here as TORCH_CHECK(false, ...) throws.

ljk53 · 2020-11-13T20:21:50Z

aten/src/ATen/native/metal/MetalPrepackOpRegister.cpp

  return mpscnn::copy_to_host(input);
 #else
-  TORCH_CHECK(false, "copy_to_host can only be invoked on iOS");
+  TORCH_CHECK(false, "copy_to_host can only be invoked on iOS and MacOS");
  return input;


ljk53 · 2020-11-13T20:23:35Z

caffe2/CMakeLists.txt

@@ -1195,6 +1195,18 @@ if(USE_CUDA)

 endif()

+# ---[ Metal(OSX) modification
+if(APPLE AND USE_PYTORCH_METAL)


nit: can we make 'USE_PYTORCH_METAL' only enabled when 'APPLE' is true?

USE_PYTORCH_METAL is also available on Linux. In that case, only one cpp file under aten/native/metal will be compiled. The reason is that users can still do optimize_for_mobile on Linux machines to export models for metal.

Add macosx support for metal. The supported os version is 10.13 and above. Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)! [ghstack-poisoned]

Pull Request resolved: #47635 Add macosx support for metal. The supported os version is 10.13 and above. ghstack-source-id: 116664268 Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)!

Add macosx support for metal. The supported os version is 10.13 and above. Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)! [ghstack-poisoned]

ljk53

accept to unblock - we could clean up these bool conditions (C10_IOS || TARGET_OS_MAC) later if they are going to be used a lot in the codebase in the future.

Add macosx support for metal. The supported os version is 10.13 and above. Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)! [ghstack-poisoned]

facebook-github-bot · 2020-11-18T01:12:39Z

This pull request has been merged in 3846e35.

PeterL1n · 2020-11-27T02:07:29Z

Does this mean PyTorch will support inferencing and training on Mac using GPU in the future?

leovinus2001 · 2020-12-07T19:35:07Z

Does this mean PyTorch will support inferencing and training on Mac using GPU in the future?

I am wondering the same. Could someone in the know write a few sentences pls as to the goal here?
Train vs inference?
macOS vs iOS?
Metal throughout PyTorch or just ATen (based on checkins I see :)
Metal just for CNN or also Linear/LSTM?

When more clarity is provided then it is easier to see whether we can contribute :)

Pull Request resolved: pytorch/pytorch#47635 Add macosx support for metal. The supported os version is 10.13 and above. ghstack-source-id: 116607205 Differential Revision: [D24825088](https://our.internmc.facebook.com/intern/diff/D24825088/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D24825088/)!

facebook-github-bot added the cla signed label Nov 9, 2020

xta0 mentioned this pull request Nov 9, 2020

[GPU] Make permuteWeights inline #47634

Closed

xta0 mentioned this pull request Nov 11, 2020

[Person Seg] Compress the person seg model #47733

Closed

xta0 mentioned this pull request Nov 11, 2020

[Segmentation] Convert the C2 model to PyTorch #47804

Closed

xta0 requested a review from husthyc November 12, 2020 07:31

xta0 added 2 commits November 12, 2020 22:03

xta0 requested a review from ljk53 November 13, 2020 09:35

ljk53 reviewed Nov 13, 2020

View reviewed changes

xta0 mentioned this pull request Nov 16, 2020

[Person Seg] Compress the person seg model #48008

Closed

ljk53 approved these changes Nov 16, 2020

View reviewed changes

facebook-github-bot closed this in 3846e35 Nov 17, 2020

facebook-github-bot added the Merged label Nov 18, 2020

facebook-github-bot deleted the gh/xta0/70/head branch November 21, 2020 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GPU] Enable Metal on macosx #47635

[GPU] Enable Metal on macosx #47635

xta0 commented Nov 9, 2020 •

edited

dr-ci bot commented Nov 10, 2020 •

edited

ljk53 left a comment

ljk53 Nov 13, 2020

ljk53 Nov 13, 2020

xta0 Nov 14, 2020

ljk53 Nov 13, 2020

ljk53 Nov 13, 2020

ljk53 Nov 13, 2020

xta0 Nov 14, 2020 •

edited

ljk53 left a comment

facebook-github-bot commented Nov 18, 2020

PeterL1n commented Nov 27, 2020

leovinus2001 commented Dec 7, 2020 •

edited

[GPU] Enable Metal on macosx #47635

[GPU] Enable Metal on macosx #47635

Conversation

xta0 commented Nov 9, 2020 • edited

dr-ci bot commented Nov 10, 2020 • edited

💊 CI failures summary and remediations

🚧 6 ongoing upstream failures:

ci.pytorch.org: 2 failed

ljk53 left a comment

Choose a reason for hiding this comment

ljk53 Nov 13, 2020

Choose a reason for hiding this comment

ljk53 Nov 13, 2020

Choose a reason for hiding this comment

xta0 Nov 14, 2020

Choose a reason for hiding this comment

ljk53 Nov 13, 2020

Choose a reason for hiding this comment

ljk53 Nov 13, 2020

Choose a reason for hiding this comment

ljk53 Nov 13, 2020

Choose a reason for hiding this comment

xta0 Nov 14, 2020 • edited

Choose a reason for hiding this comment

ljk53 left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Nov 18, 2020

PeterL1n commented Nov 27, 2020

leovinus2001 commented Dec 7, 2020 • edited

xta0 commented Nov 9, 2020 •

edited

dr-ci bot commented Nov 10, 2020 •

edited

xta0 Nov 14, 2020 •

edited

leovinus2001 commented Dec 7, 2020 •

edited