-
Notifications
You must be signed in to change notification settings - Fork 531
CoreML Partitioner is not able to lower mv3 #10451
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
Comments
Do we have something close to this in CI? Like a quantizer variant perhaps? |
@shoumikhin had to disable dim order https://github.com/pytorch-labs/executorch-examples/pull/23/files when exporting |
I took a closer look. When dim order is enabled (now the default), this model has “executorch.exir.dialects.edge._ops.dim_order_ops._to_dim_order_copy.default” ops that return floats, and this op is not recognized by CoreML (https://github.com/apple/coremltools/blob/main/coremltools/converters/mil/frontend/torch/ops.py), so the partitioner skips them. But this results in a delegate call that passes in a bunch of floats as inputs. coremltools wraps these floats in Rank1 tensors at compile time, but to ExecuTorch they are still floats. At runtime, ExecuTorch forwards these floats to the CoreML delegate, but the model complains it hasn't received the wrapped tensor inputs (which results in an error). When dim order is disabled, we see “executorch.exir.dialects.edge._ops.aten._to_copy.default” instead in the graph and this is supported by CoreML and grabbed by the partitioner. So the delegate only has a tensor input and things work fine. Disabled dim order for this model is a short term fix. Longer term, we should 1) add support for _to_dim_order_copy to coremltools, and 2) handle scalars in ET CoreML’s runtime in the same way they’re handled by coremltools at compile time (i.e., wrap them in rank 1 tensors). Either one of these fixes would solve the problem for this model, but we should probably do both. |
In terms of why CI did not catch this when dim order was enabled by default, it does not look like we use partitioner in our test_models.sh script. We use the older to_backend API instead. To use partitioner, this arg needs to be set: https://github.com/pytorch/executorch/blob/main/examples/apple/coreml/scripts/export.py#L76 |
|
But that doesn't support the dim order op, so the partitioner will still skip it. |
coremltools wraps rank 0 tensors as rank1 tensors in the AOT flow when producing a PTE file. This PR: * Applies the same change on the coreml runtime * Changes CoreML model tests to use the partitioner Fixes #10451
🐛 Describe the bug
Even though it is able to generate a file, it is spewing so much error. And during runtime it is crashing.
https://gist.github.com/mergennachin/74ca8ef593bc6c962d8d1baacaede2ed
On the other hand,
is fine because it has many layers of patches to make CoreML work
Versions
executorch==0.6.0
cc @kimishpatel @YifanShenSZ @cymbalrush @metascroy @byjlw
The text was updated successfully, but these errors were encountered: