Separate CoreML util and inmemoryfs targets to be reusable by jathu · Pull Request #9481 · pytorch/executorch

jathu · 2025-03-21T00:47:43Z

Summary

In this diff, we separate the targets for coreml/runtime/util and coreml/runtime/inmemoryfs.

Why?

As part of #9019, we want to support CoreML export out-of-the-box. Unfortunately, part of the workflow now includes installing an executorch+coreml binding as a pip wheel:

executorch/backends/apple/coreml/scripts/install_inmemoryfs.sh

Line 20 in 5fdfa51

pip install "$COREML_DIR_PATH/runtime/inmemoryfs"

This then gets used throughout the library. i.e.

executorch/backends/apple/coreml/compiler/coreml_preprocess.py

Line 19 in 5fdfa51

importexecutorchcoreml

So we want to now bake this pybinding into the main wheel. Ultimately this executorchcoreml module, gets built using inmemoryfs:

executorch/backends/apple/coreml/runtime/inmemoryfs/setup.py

Lines 17 to 38 in 5fdfa51

ext_modules= [ 
Pybind11Extension( 
"executorchcoreml", 
 [ 
"../util/json_util.cpp", 
"inmemory_filesystem.cpp", 
"inmemory_filesystem_py.cpp", 
"inmemory_filesystem_utils.cpp", 
"memory_buffer.cpp", 
"memory_stream.cpp", 
"reversed_memory_stream.cpp", 
 ], 
define_macros=[("VERSION_INFO", __version__)], 
cxx_std=cxx_std, 
extra_compile_args=["-mmacosx-version-min=10.15", "-g"], 
include_dirs=[ 
"../../third-party/nlohmann_json/single_include", 
".", 
"../util", 
 ], 
 ), 
 ] 

So, to enable creating a pybinding target, we must first decouple the util and inmemoryfs libraries so they can be used in coremldelegate and the future pybinding.

Test plan

This should really be a no-op w.r.t. builds. So, CI +

$ ./install_executorch.sh --pybind coreml

cc @larryliu0820 @lucylq

pytorch-bot · 2025-03-21T00:47:47Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

⏳ No Failures, 6 Pending

As of commit 6c82a26 with merge base 06ec67c ():
💚 Looks good so far! There are no failures yet. 💚

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

mergennachin

See inline comments

swolchok

seems fine, leaving accept for @shoumikhin

shoumikhin · 2025-03-24T19:45:35Z

LGTM and I'd also asked @cymbalrush to glance

This reverts commit a03a108.

@larryliu0820

### Summary Context: #9481 We now create the pybinding target for inmemoryfs. Note that the build recipe is from the wheel builder: https://github.com/pytorch/executorch/blob/5fdfa511966208c7e237a9e920a7c63f513b4fb7/backends/apple/coreml/runtime/inmemoryfs/setup.py#L17-L38 ### Test plan ``` $ cmake -B cmake-out -S . -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON $ cmake --build cmake-out -j$(sysctl -n hw.ncpu) --target executorchcoreml $ ./install_executorch.sh --pybind coreml ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

jathu · 2025-03-21T00:47:43Z

Summary

In this diff, we separate the targets for coreml/runtime/util and coreml/runtime/inmemoryfs.

Why?

As part of #9019, we want to support CoreML export out-of-the-box. Unfortunately, part of the workflow now includes installing an executorch+coreml binding as a pip wheel:

executorch/backends/apple/coreml/scripts/install_inmemoryfs.sh

Line 20 in 5fdfa51

pip install "$COREML_DIR_PATH/runtime/inmemoryfs"

This then gets used throughout the library. i.e.

executorch/backends/apple/coreml/compiler/coreml_preprocess.py

Line 19 in 5fdfa51

importexecutorchcoreml

So we want to now bake this pybinding into the main wheel. Ultimately this executorchcoreml module, gets built using inmemoryfs:

executorch/backends/apple/coreml/runtime/inmemoryfs/setup.py

Lines 17 to 38 in 5fdfa51

ext_modules= [ 
Pybind11Extension( 
"executorchcoreml", 
 [ 
"../util/json_util.cpp", 
"inmemory_filesystem.cpp", 
"inmemory_filesystem_py.cpp", 
"inmemory_filesystem_utils.cpp", 
"memory_buffer.cpp", 
"memory_stream.cpp", 
"reversed_memory_stream.cpp", 
 ], 
define_macros=[("VERSION_INFO", __version__)], 
cxx_std=cxx_std, 
extra_compile_args=["-mmacosx-version-min=10.15", "-g"], 
include_dirs=[ 
"../../third-party/nlohmann_json/single_include", 
".", 
"../util", 
 ], 
 ), 
 ] 

So, to enable creating a pybinding target, we must first decouple the util and inmemoryfs libraries so they can be used in coremldelegate and the future pybinding.

Test plan

This should really be a no-op w.r.t. builds. So, CI +

$ ./install_executorch.sh --pybind coreml

cc @larryliu0820 @lucylq

pytorch-bot · 2025-03-21T00:47:47Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

⏳ No Failures, 6 Pending

As of commit 6c82a26 with merge base 06ec67c ():
💚 Looks good so far! There are no failures yet. 💚

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

mergennachin

See inline comments

swolchok

seems fine, leaving accept for @shoumikhin

shoumikhin · 2025-03-24T19:45:35Z

LGTM and I'd also asked @cymbalrush to glance

This reverts commit a03a108.

@larryliu0820

### Summary Context: #9481 We now create the pybinding target for inmemoryfs. Note that the build recipe is from the wheel builder: https://github.com/pytorch/executorch/blob/5fdfa511966208c7e237a9e920a7c63f513b4fb7/backends/apple/coreml/runtime/inmemoryfs/setup.py#L17-L38 ### Test plan ``` $ cmake -B cmake-out -S . -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON $ cmake --build cmake-out -j$(sysctl -n hw.ncpu) --target executorchcoreml $ ./install_executorch.sh --pybind coreml ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

jathu · 2025-03-21T00:47:43Z

Summary

In this diff, we separate the targets for coreml/runtime/util and coreml/runtime/inmemoryfs.

Why?

As part of #9019, we want to support CoreML export out-of-the-box. Unfortunately, part of the workflow now includes installing an executorch+coreml binding as a pip wheel:

executorch/backends/apple/coreml/scripts/install_inmemoryfs.sh

Line 20 in 5fdfa51

pip install "$COREML_DIR_PATH/runtime/inmemoryfs"

This then gets used throughout the library. i.e.

executorch/backends/apple/coreml/compiler/coreml_preprocess.py

Line 19 in 5fdfa51

importexecutorchcoreml

So we want to now bake this pybinding into the main wheel. Ultimately this executorchcoreml module, gets built using inmemoryfs:

executorch/backends/apple/coreml/runtime/inmemoryfs/setup.py

Lines 17 to 38 in 5fdfa51

ext_modules= [ 
Pybind11Extension( 
"executorchcoreml", 
 [ 
"../util/json_util.cpp", 
"inmemory_filesystem.cpp", 
"inmemory_filesystem_py.cpp", 
"inmemory_filesystem_utils.cpp", 
"memory_buffer.cpp", 
"memory_stream.cpp", 
"reversed_memory_stream.cpp", 
 ], 
define_macros=[("VERSION_INFO", __version__)], 
cxx_std=cxx_std, 
extra_compile_args=["-mmacosx-version-min=10.15", "-g"], 
include_dirs=[ 
"../../third-party/nlohmann_json/single_include", 
".", 
"../util", 
 ], 
 ), 
 ] 

So, to enable creating a pybinding target, we must first decouple the util and inmemoryfs libraries so they can be used in coremldelegate and the future pybinding.

Test plan

This should really be a no-op w.r.t. builds. So, CI +

$ ./install_executorch.sh --pybind coreml

cc @larryliu0820 @lucylq

pytorch-bot · 2025-03-21T00:47:47Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

⏳ No Failures, 6 Pending

As of commit 6c82a26 with merge base 06ec67c ():
💚 Looks good so far! There are no failures yet. 💚

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

mergennachin

See inline comments

swolchok

seems fine, leaving accept for @shoumikhin

shoumikhin · 2025-03-24T19:45:35Z

LGTM and I'd also asked @cymbalrush to glance

This reverts commit a03a108.

@larryliu0820

### Summary Context: #9481 We now create the pybinding target for inmemoryfs. Note that the build recipe is from the wheel builder: https://github.com/pytorch/executorch/blob/5fdfa511966208c7e237a9e920a7c63f513b4fb7/backends/apple/coreml/runtime/inmemoryfs/setup.py#L17-L38 ### Test plan ``` $ cmake -B cmake-out -S . -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON $ cmake --build cmake-out -j$(sysctl -n hw.ncpu) --target executorchcoreml $ ./install_executorch.sh --pybind coreml ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

jathu · 2025-03-21T00:47:43Z

Summary

In this diff, we separate the targets for coreml/runtime/util and coreml/runtime/inmemoryfs.

Why?

As part of #9019, we want to support CoreML export out-of-the-box. Unfortunately, part of the workflow now includes installing an executorch+coreml binding as a pip wheel:

executorch/backends/apple/coreml/scripts/install_inmemoryfs.sh

Line 20 in 5fdfa51

pip install "$COREML_DIR_PATH/runtime/inmemoryfs"

This then gets used throughout the library. i.e.

executorch/backends/apple/coreml/compiler/coreml_preprocess.py

Line 19 in 5fdfa51

importexecutorchcoreml

So we want to now bake this pybinding into the main wheel. Ultimately this executorchcoreml module, gets built using inmemoryfs:

executorch/backends/apple/coreml/runtime/inmemoryfs/setup.py

Lines 17 to 38 in 5fdfa51

ext_modules= [ 
Pybind11Extension( 
"executorchcoreml", 
 [ 
"../util/json_util.cpp", 
"inmemory_filesystem.cpp", 
"inmemory_filesystem_py.cpp", 
"inmemory_filesystem_utils.cpp", 
"memory_buffer.cpp", 
"memory_stream.cpp", 
"reversed_memory_stream.cpp", 
 ], 
define_macros=[("VERSION_INFO", __version__)], 
cxx_std=cxx_std, 
extra_compile_args=["-mmacosx-version-min=10.15", "-g"], 
include_dirs=[ 
"../../third-party/nlohmann_json/single_include", 
".", 
"../util", 
 ], 
 ), 
 ] 

So, to enable creating a pybinding target, we must first decouple the util and inmemoryfs libraries so they can be used in coremldelegate and the future pybinding.

Test plan

This should really be a no-op w.r.t. builds. So, CI +

$ ./install_executorch.sh --pybind coreml

cc @larryliu0820 @lucylq

pytorch-bot · 2025-03-21T00:47:47Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

⏳ No Failures, 6 Pending

As of commit 6c82a26 with merge base 06ec67c ():
💚 Looks good so far! There are no failures yet. 💚

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

mergennachin

See inline comments

swolchok

seems fine, leaving accept for @shoumikhin

shoumikhin · 2025-03-24T19:45:35Z

LGTM and I'd also asked @cymbalrush to glance

This reverts commit a03a108.

@larryliu0820

### Summary Context: #9481 We now create the pybinding target for inmemoryfs. Note that the build recipe is from the wheel builder: https://github.com/pytorch/executorch/blob/5fdfa511966208c7e237a9e920a7c63f513b4fb7/backends/apple/coreml/runtime/inmemoryfs/setup.py#L17-L38 ### Test plan ``` $ cmake -B cmake-out -S . -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON $ cmake --build cmake-out -j$(sysctl -n hw.ncpu) --target executorchcoreml $ ./install_executorch.sh --pybind coreml ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

jathu · 2025-03-21T00:47:43Z

Summary

In this diff, we separate the targets for coreml/runtime/util and coreml/runtime/inmemoryfs.

Why?

As part of #9019, we want to support CoreML export out-of-the-box. Unfortunately, part of the workflow now includes installing an executorch+coreml binding as a pip wheel:

executorch/backends/apple/coreml/scripts/install_inmemoryfs.sh

Line 20 in 5fdfa51

pip install "$COREML_DIR_PATH/runtime/inmemoryfs"

This then gets used throughout the library. i.e.

executorch/backends/apple/coreml/compiler/coreml_preprocess.py

Line 19 in 5fdfa51

importexecutorchcoreml

So we want to now bake this pybinding into the main wheel. Ultimately this executorchcoreml module, gets built using inmemoryfs:

executorch/backends/apple/coreml/runtime/inmemoryfs/setup.py

Lines 17 to 38 in 5fdfa51

ext_modules= [ 
Pybind11Extension( 
"executorchcoreml", 
 [ 
"../util/json_util.cpp", 
"inmemory_filesystem.cpp", 
"inmemory_filesystem_py.cpp", 
"inmemory_filesystem_utils.cpp", 
"memory_buffer.cpp", 
"memory_stream.cpp", 
"reversed_memory_stream.cpp", 
 ], 
define_macros=[("VERSION_INFO", __version__)], 
cxx_std=cxx_std, 
extra_compile_args=["-mmacosx-version-min=10.15", "-g"], 
include_dirs=[ 
"../../third-party/nlohmann_json/single_include", 
".", 
"../util", 
 ], 
 ), 
 ] 

So, to enable creating a pybinding target, we must first decouple the util and inmemoryfs libraries so they can be used in coremldelegate and the future pybinding.

Test plan

This should really be a no-op w.r.t. builds. So, CI +

$ ./install_executorch.sh --pybind coreml

cc @larryliu0820 @lucylq

pytorch-bot · 2025-03-21T00:47:47Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

⏳ No Failures, 6 Pending

As of commit 6c82a26 with merge base 06ec67c ():
💚 Looks good so far! There are no failures yet. 💚

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

mergennachin

See inline comments

swolchok

seems fine, leaving accept for @shoumikhin

shoumikhin · 2025-03-24T19:45:35Z

LGTM and I'd also asked @cymbalrush to glance

This reverts commit a03a108.

@larryliu0820

### Summary Context: #9481 We now create the pybinding target for inmemoryfs. Note that the build recipe is from the wheel builder: https://github.com/pytorch/executorch/blob/5fdfa511966208c7e237a9e920a7c63f513b4fb7/backends/apple/coreml/runtime/inmemoryfs/setup.py#L17-L38 ### Test plan ``` $ cmake -B cmake-out -S . -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON $ cmake --build cmake-out -j$(sysctl -n hw.ncpu) --target executorchcoreml $ ./install_executorch.sh --pybind coreml ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

jathu · 2025-03-21T00:47:43Z

Summary

In this diff, we separate the targets for coreml/runtime/util and coreml/runtime/inmemoryfs.

Why?

As part of #9019, we want to support CoreML export out-of-the-box. Unfortunately, part of the workflow now includes installing an executorch+coreml binding as a pip wheel:

executorch/backends/apple/coreml/scripts/install_inmemoryfs.sh

Line 20 in 5fdfa51

pip install "$COREML_DIR_PATH/runtime/inmemoryfs"

This then gets used throughout the library. i.e.

executorch/backends/apple/coreml/compiler/coreml_preprocess.py

Line 19 in 5fdfa51

importexecutorchcoreml

So we want to now bake this pybinding into the main wheel. Ultimately this executorchcoreml module, gets built using inmemoryfs:

executorch/backends/apple/coreml/runtime/inmemoryfs/setup.py

Lines 17 to 38 in 5fdfa51

ext_modules= [ 
Pybind11Extension( 
"executorchcoreml", 
 [ 
"../util/json_util.cpp", 
"inmemory_filesystem.cpp", 
"inmemory_filesystem_py.cpp", 
"inmemory_filesystem_utils.cpp", 
"memory_buffer.cpp", 
"memory_stream.cpp", 
"reversed_memory_stream.cpp", 
 ], 
define_macros=[("VERSION_INFO", __version__)], 
cxx_std=cxx_std, 
extra_compile_args=["-mmacosx-version-min=10.15", "-g"], 
include_dirs=[ 
"../../third-party/nlohmann_json/single_include", 
".", 
"../util", 
 ], 
 ), 
 ] 

So, to enable creating a pybinding target, we must first decouple the util and inmemoryfs libraries so they can be used in coremldelegate and the future pybinding.

Test plan

This should really be a no-op w.r.t. builds. So, CI +

$ ./install_executorch.sh --pybind coreml

cc @larryliu0820 @lucylq

pytorch-bot · 2025-03-21T00:47:47Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

⏳ No Failures, 6 Pending

As of commit 6c82a26 with merge base 06ec67c ():
💚 Looks good so far! There are no failures yet. 💚

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

mergennachin

See inline comments

swolchok

seems fine, leaving accept for @shoumikhin

shoumikhin · 2025-03-24T19:45:35Z

LGTM and I'd also asked @cymbalrush to glance

This reverts commit a03a108.

@larryliu0820

### Summary Context: #9481 We now create the pybinding target for inmemoryfs. Note that the build recipe is from the wheel builder: https://github.com/pytorch/executorch/blob/5fdfa511966208c7e237a9e920a7c63f513b4fb7/backends/apple/coreml/runtime/inmemoryfs/setup.py#L17-L38 ### Test plan ``` $ cmake -B cmake-out -S . -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON $ cmake --build cmake-out -j$(sysctl -n hw.ncpu) --target executorchcoreml $ ./install_executorch.sh --pybind coreml ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

jathu · 2025-03-21T00:47:43Z

Summary

In this diff, we separate the targets for coreml/runtime/util and coreml/runtime/inmemoryfs.

Why?

As part of #9019, we want to support CoreML export out-of-the-box. Unfortunately, part of the workflow now includes installing an executorch+coreml binding as a pip wheel:

executorch/backends/apple/coreml/scripts/install_inmemoryfs.sh

Line 20 in 5fdfa51

pip install "$COREML_DIR_PATH/runtime/inmemoryfs"

This then gets used throughout the library. i.e.

executorch/backends/apple/coreml/compiler/coreml_preprocess.py

Line 19 in 5fdfa51

importexecutorchcoreml

So we want to now bake this pybinding into the main wheel. Ultimately this executorchcoreml module, gets built using inmemoryfs:

executorch/backends/apple/coreml/runtime/inmemoryfs/setup.py

Lines 17 to 38 in 5fdfa51

ext_modules= [ 
Pybind11Extension( 
"executorchcoreml", 
 [ 
"../util/json_util.cpp", 
"inmemory_filesystem.cpp", 
"inmemory_filesystem_py.cpp", 
"inmemory_filesystem_utils.cpp", 
"memory_buffer.cpp", 
"memory_stream.cpp", 
"reversed_memory_stream.cpp", 
 ], 
define_macros=[("VERSION_INFO", __version__)], 
cxx_std=cxx_std, 
extra_compile_args=["-mmacosx-version-min=10.15", "-g"], 
include_dirs=[ 
"../../third-party/nlohmann_json/single_include", 
".", 
"../util", 
 ], 
 ), 
 ] 

So, to enable creating a pybinding target, we must first decouple the util and inmemoryfs libraries so they can be used in coremldelegate and the future pybinding.

Test plan

This should really be a no-op w.r.t. builds. So, CI +

$ ./install_executorch.sh --pybind coreml

cc @larryliu0820 @lucylq

pytorch-bot · 2025-03-21T00:47:47Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

⏳ No Failures, 6 Pending

As of commit 6c82a26 with merge base 06ec67c ():
💚 Looks good so far! There are no failures yet. 💚

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

mergennachin

See inline comments

swolchok

seems fine, leaving accept for @shoumikhin

shoumikhin · 2025-03-24T19:45:35Z

LGTM and I'd also asked @cymbalrush to glance

This reverts commit a03a108.

@larryliu0820

### Summary Context: #9481 We now create the pybinding target for inmemoryfs. Note that the build recipe is from the wheel builder: https://github.com/pytorch/executorch/blob/5fdfa511966208c7e237a9e920a7c63f513b4fb7/backends/apple/coreml/runtime/inmemoryfs/setup.py#L17-L38 ### Test plan ``` $ cmake -B cmake-out -S . -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON $ cmake --build cmake-out -j$(sysctl -n hw.ncpu) --target executorchcoreml $ ./install_executorch.sh --pybind coreml ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

jathu · 2025-03-21T00:47:43Z

Summary

In this diff, we separate the targets for coreml/runtime/util and coreml/runtime/inmemoryfs.

Why?

As part of #9019, we want to support CoreML export out-of-the-box. Unfortunately, part of the workflow now includes installing an executorch+coreml binding as a pip wheel:

executorch/backends/apple/coreml/scripts/install_inmemoryfs.sh

Line 20 in 5fdfa51

pip install "$COREML_DIR_PATH/runtime/inmemoryfs"

This then gets used throughout the library. i.e.

executorch/backends/apple/coreml/compiler/coreml_preprocess.py

Line 19 in 5fdfa51

importexecutorchcoreml

So we want to now bake this pybinding into the main wheel. Ultimately this executorchcoreml module, gets built using inmemoryfs:

executorch/backends/apple/coreml/runtime/inmemoryfs/setup.py

Lines 17 to 38 in 5fdfa51

ext_modules= [ 
Pybind11Extension( 
"executorchcoreml", 
 [ 
"../util/json_util.cpp", 
"inmemory_filesystem.cpp", 
"inmemory_filesystem_py.cpp", 
"inmemory_filesystem_utils.cpp", 
"memory_buffer.cpp", 
"memory_stream.cpp", 
"reversed_memory_stream.cpp", 
 ], 
define_macros=[("VERSION_INFO", __version__)], 
cxx_std=cxx_std, 
extra_compile_args=["-mmacosx-version-min=10.15", "-g"], 
include_dirs=[ 
"../../third-party/nlohmann_json/single_include", 
".", 
"../util", 
 ], 
 ), 
 ] 

So, to enable creating a pybinding target, we must first decouple the util and inmemoryfs libraries so they can be used in coremldelegate and the future pybinding.

Test plan

This should really be a no-op w.r.t. builds. So, CI +

$ ./install_executorch.sh --pybind coreml

cc @larryliu0820 @lucylq

pytorch-bot · 2025-03-21T00:47:47Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

⏳ No Failures, 6 Pending

As of commit 6c82a26 with merge base 06ec67c ():
💚 Looks good so far! There are no failures yet. 💚

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

mergennachin

See inline comments

swolchok

seems fine, leaving accept for @shoumikhin

shoumikhin · 2025-03-24T19:45:35Z

LGTM and I'd also asked @cymbalrush to glance

This reverts commit a03a108.

@larryliu0820

### Summary Context: #9481 We now create the pybinding target for inmemoryfs. Note that the build recipe is from the wheel builder: https://github.com/pytorch/executorch/blob/5fdfa511966208c7e237a9e920a7c63f513b4fb7/backends/apple/coreml/runtime/inmemoryfs/setup.py#L17-L38 ### Test plan ``` $ cmake -B cmake-out -S . -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON $ cmake --build cmake-out -j$(sysctl -n hw.ncpu) --target executorchcoreml $ ./install_executorch.sh --pybind coreml ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

@larryliu0820

### Summary Context: #9481 * Include the `executorchcoreml` pybinding in the builds * Remove separate installation option * Turn on CoreML by default for macOS builds * Add a dependency on coremltools for macOS ### Test plan CI ``` $ rm -rf cmake-out pip-out dist && ./install_executorch.sh $ ./examples/models/llama/install_requirements.sh $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode coreml $ .ci/scripts/test_llama.sh -model stories110M -build_tool cmake -dtype fp32 -mode xnnpack+custom+quantize_kv ``` cc @larryliu0820 @lucylq

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 21, 2025

jathu requested review from larryliu0820, mergennachin and swolchok March 21, 2025 00:48

jathu added module: build/install Issues related to the cmake and buck2 builds, and to installing ExecuTorch ciflow/trunk topic: not user facing labels Mar 21, 2025

jathu marked this pull request as ready for review March 21, 2025 00:49

jathu requested review from cccclai and shoumikhin as code owners March 21, 2025 00:49

jathu mentioned this pull request Mar 21, 2025
Create a CoreML inmemoryfs pybinding target #9482
Merged

larryliu0820 approved these changes Mar 21, 2025
View reviewed changes

jathu removed the request for review from cccclai March 21, 2025 01:04

jathu mentioned this pull request Mar 21, 2025
Enable CoreML by default on macOS wheel builds #9483
Merged

cccclai requested review from YifanShenSZ and cymbalrush March 21, 2025 04:05

mergennachin approved these changes Mar 21, 2025
View reviewed changes

Comment threadbackends/apple/coreml/CMakeLists.txt
Comment threadbackends/apple/coreml/CMakeLists.txt Outdated
Comment threadbackends/apple/coreml/CMakeLists.txt Outdated
Comment threadbackends/apple/coreml/CMakeLists.txt Outdated

jathu force-pushed the jathu/split-coreml branch 2 times, most recently from 1f3966d to 05b129aCompare March 21, 2025 14:52

mergennachin approved these changes Mar 21, 2025
View reviewed changes

jathu force-pushed the jathu/split-coreml branch 4 times, most recently from d9f8ff5 to 639128bCompare March 21, 2025 19:11

jathu requested a review from mergennachin March 21, 2025 19:11

jathu commented Mar 21, 2025
View reviewed changes

Comment threadbackends/apple/coreml/.gitignore Outdated

jathu force-pushed the jathu/split-coreml branch from 639128b to 7f6cdc3Compare March 21, 2025 19:13

jathu marked this pull request as draft March 21, 2025 19:32

jathu force-pushed the jathu/split-coreml branch from 7f6cdc3 to a03a108Compare March 21, 2025 19:41

jathu force-pushed the jathu/split-coreml branch 4 times, most recently from 3172a23 to ecbceb5Compare March 23, 2025 22:28

jathu marked this pull request as ready for review March 23, 2025 22:53

jathu requested review from larryliu0820 and mergennachin March 23, 2025 22:53

jathu force-pushed the jathu/split-coreml branch 2 times, most recently from 067d471 to fdc8ffcCompare March 24, 2025 16:48

swolchok reviewed Mar 24, 2025
View reviewed changes

Comment threadbackends/apple/coreml/CMakeLists.txt Outdated

jathu force-pushed the jathu/split-coreml branch from fdc8ffc to b0a8f96Compare March 24, 2025 18:14

jathu requested a review from metascroy as a code owner March 24, 2025 18:14

jathu force-pushed the jathu/split-coreml branch 2 times, most recently from 43afb06 to edb05ceCompare March 24, 2025 18:43

cymbalrush approved these changes Mar 24, 2025
View reviewed changes

shoumikhin approved these changes Mar 24, 2025
View reviewed changes

jathu force-pushed the jathu/split-coreml branch from edb05ce to f96e713Compare March 24, 2025 20:48

jathu requested a review from GregoryComer as a code owner March 24, 2025 20:48

jathu added 5 commits March 24, 2025 15:45

split targets
e9d771b

address comments
e6e999d

attempt to fix ci
9e2e845

Revert "attempt to fix ci"
e2b9171
This reverts commit a03a108.

attempt to fix linker
6c82a26

jathu force-pushed the jathu/split-coreml branch from f96e713 to 6c82a26Compare March 24, 2025 22:45

jathu merged commit cb583f3 into mainMar 24, 2025

jathu deleted the jathu/split-coreml branch March 24, 2025 23:56