Skip to content

Core ML: run ops that read uint8 and int8 inputs on the CPU, refuse ops coremltools gets wrong, and fix fills, tanh GELU, RMSNorm and repeated outputs - #24317

Open
shoumikhin wants to merge 38 commits into
pytorch:mainfrom
shoumikhin:coreml-uint8-inputs
Open

shoumikhin wants to merge 38 commits into
pytorch:mainfrom
shoumikhin:coreml-uint8-inputs

Conversation

@shoumikhin

@shoumikhin shoumikhin commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Core ML lowering crashes, or gives wrong values, for several common patterns. This change fixes them in the Core ML partitioner and backend, plus an exir (ExecuTorch export library) bug that the new partitioner refusals run into.

Problems and fixes

Lowering crashes, or the program fails to load:

  • uint8 and int8 inputs. coremltools cannot take them as program inputs, so converting a uint8 camera frame to float fails with KeyError: torch.uint8 (issue CoreML partitioner errors out with 8-bit integer types #11686). Ops that read such an input now run on the CPU up to the conversion to float, and the ops after it still go to Core ML. So does an op that makes an int8 tensor used outside Core ML, which the Core ML runtime cannot return, and a partition left with only size inputs, or no inputs below iOS 18. With lower_full_graph=True, lowering fails and names the tensor.
  • State. If a CPU op splits the users of a state buffer that Core ML takes over, lowering now fails, names the ops and where they run, and points to take_over_mutable_buffer=False, instead of leaving one side with a stale copy. State that only CPU ops read stays on the CPU, also when Core ML computes its new value.
  • bool min and max. min, max, amin, amax, minimum or maximum over a bool tensor, such as the check torch.distributions.MultivariateNormal exports, crashes lowering. These ops now run on the portable CPU kernels.
  • Kept ops rejected after decomposition. When a float32 matmul keeps aten.matmul whole, a bfloat16 linear decomposes into another matmul that also stays whole, on the CPU, and loading fails with "kernel 'aten::matmul.out' not found". The partitioner now decomposes a copy of the program the way lowering will and runs the same search there, so an op that ends up on the CPU is not kept whole (an op skipped by name still is). This dry run only happens with a uint8 or int8 tensor, or when an op Core ML rejects gets decomposed, and once per program.
  • Fills. coremltools makes full without a dtype, zeros and new_zeros as float, ones_like as float from iOS 16, zeros_like with the dtype of its input, and full_like, new_full, fill and scalar_tensor with the type of their fill value. So torch.full((1, 16), 0) (int64 in torch) is made as float, and index math on it fails to lower, as in the SmolVLM vision embeddings, or loses integers above 2048 at float16 precision. The backend now converts these ops, and empty, new_empty and empty_like, with the dtype torch gives them, so they stay in Core ML. A fill value known only at run time keeps the coremltools conversion.
  • RMSNorm without eps. torch.nn.RMSNorm (root mean square normalization) leaves out eps (the small number added before the square root) by default, and coremltools requires it: "got 3 input(s), expected [4]". The backend now passes the float32 machine epsilon, which is what PyTorch uses. A model whose tensors are float16 still fails inside coremltools, with or without eps.
  • index_put with a whole slice. coremltools converts y[:, idx] = v only when idx has a single entry. With idx = torch.tensor([0, 2]), as in the X-VLA policy, lowering fails with "Input squeeze_0 has rank 1 != other inputs rank 0". accumulate=True, a bool mask, or, below iOS 18, a bool tensor next to a slice fail too, and so do index tensors that broadcast. These forms now run on the portable kernels, while forms coremltools handles, such as a single-entry index for a cache update, stay in Core ML. So a cache update at several positions, as in a prefill, now lowers, with the update on the CPU. A negative index next to a slice still stays in Core ML, since the partitioner cannot see index values: it fails, or, for an index computed at run time, can give wrong values.
  • A shared constant read from a list. When ops in a delegate and on the CPU read one constant, to_backend gives each side a copy, but it missed ops that read it from a list, such as cat or index_put. Lowering then failed with "constant data node ... is tagged with ... but has user ... which has tag (None)". duplicate_constant_node in exir now replaces the constant there too.

Wrong values without any error:

  • tanh GELU. gelu(approximate="tanh") ran as the exact GELU (Gaussian Error Linear Unit), because torch.export passes approximate as a keyword and coremltools reads it only from the positional arguments. The two differ by up to 4.7e-4, which in pi0 and pi0.5 was the whole float32 error of the policy. The backend now reads the keyword.
  • An output that repeats an input or another output. coremltools turns ops such as clone into nothing, so a partition can return its own input, as in return x.clone(), x * 2, or one value twice. Next to other outputs, such an output came back as zeros or as the bytes of another output. The backend now gives each one a copy of its own.
  • index_put with a 0-d index or several indices next to a slice. coremltools lays out the written part differently from torch, so values land in the wrong place unless they vary only in the dims after the last index. Those forms now run on the portable kernels.
  • nn.MultiheadAttention with a bool mask. It turns the mask into a float one with torch.zeros_like(mask, dtype=...), which coremltools kept bool, so attention came out wrong. The fill fix above covers this too.

With lower_full_graph=True, an op the partitioner refuses now fails with the reason, such as "aten.index_put.default with these indices is not supported by CoreML. Set lower_full_graph=False in the CoreMLPartitioner to run it outside of CoreML.", instead of saying that Core ML does not support the op.

A uint8 buffer that the model casts still fails with the default settings, and lowers with take_over_constant_data=False, as the docs now say.

How to review

Start with coreml_partitioner.py: the uint8 and int8 search in _propose_partitions, the state check, and the dry run in _drop_ops_left_outside. The refusals are small predicates near the top of the file, one per problem. The fixes in torch_ops.py, coreml_preprocess.py and exir do not depend on the partitioner changes.

Tests

With coremltools 9.0, the tests below cover these cases. On macOS 15 or later they also run the program with the Core ML runtime and compare it with eager. With the Core ML code of main, the fill, index_put, state and compiler tests fail (the nn.MultiheadAttention case only with the runtime). The cases:

  • uint8 frames read by pad, reflect pad, max_pool2d, roll, zeros_like, masked_fill, a channel swap, max(dim=1) or split, an int8 frame read by a permute, and a roll with a dynamic height.
  • uint8 tensors next to ops left out of Core ML, such as a skipped flip or a diagonal, int8 outputs, state split by a CPU read or update, CPU-only state, state read only on the CPU, bool min/amax/maximum, and a bfloat16 linear next to a float32 matmul.
  • Every fill op above, with an int or float fill value or a dtype argument, and nn.MultiheadAttention with a bool mask; RMSNorm without eps and tanh GELU, also checked in the converted program; a returned clone and a value returned twice.
  • index_put with a two-entry index, broadcast index tensors, accumulate=True or a bool tensor written next to a slice, and 0-d indices whose values do not fit or would land in the wrong place. With lower_full_graph=True, the error gives the reason.
  • In exir, a buffer read by two cat ops gets a copy for the second one.

test_coreml_partitioner.py, test_torch_ops.py and test_coreml_recipes.py pass with the Core ML runtime on macOS, and the new exir test passes, and fails without its fix. For a transformer block and conv models, with and without skipped ops, the partitions match main.

LeRobot policies, measured on an earlier version of this change with wheels built from the 1.6 release branch: without it, ACT, Diffusion, VQ-BeT, Multi-Task DiT and pi0.5 (bfloat16) fail to lower with KeyError: torch.uint8. With it, ACT, Diffusion, VQ-BeT and Multi-Task DiT export, run without loading torch, and match eager within 4.2e-06 over 16 frames. pi0.5 exports and passes its one-chunk check, but runs very slowly, since its bfloat16 linears run on the portable CPU kernels.

Not tested: Core ML state at run time, and a model that builds torch.distributions.MultivariateNormal itself (the test copies its bool check).

@shoumikhin shoumikhin added the release notes: apple Changes to the Apple backend delegate label Oct 30, 2026
Copilot AI balanced review requested due to automatic review settings October 30, 2026 02:51
@shoumikhin
shoumikhin requested a review from metascroy as a code owner October 30, 2026 02:51
@pytorch-bot

pytorch-bot Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

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

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

⏳ No Failures, 1 Pending

As of commit c315323 with merge base 681e493 (image):
💚 Looks good so far! There are no failures yet. 💚

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@meta-cla meta-cla 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 Oct 30, 2026
@claude

claude Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Claude finished @shoumikhin's task in 28m 3s —— View job


Summary: This fixes the reported uint8 camera-frame case, but some uint8/int8 inputs still fail and the new tests leave most of the claimed behaviour unchecked. A split/unbind of the frame followed by F.pad now fails at to_executorch(), reflect/replicate pad on the frame probably still can't lower, and the new eager-match check always compares all-zero tensors. Recommendation: Request Changes.

Detailed review

PR Review: #24317

Summary

The PR keeps ops that read a uint8/int8 tensor from outside their Core ML partition on the CPU.

  • partition() re-runs CapabilityBasedPartitioner until no partition reads such a tensor.
  • ops_to_not_decompose predicts the same set of ops on the ATen graph, so that those ops are kept whole only when they have an out variant.

The problems:

  • The prediction misses cases that partition() catches.
  • Decomposing a reader can still leave a partition Core ML rejects.
  • The new tests leave most of the claimed behaviour unchecked.

Nothing was run for this review, because this job is not allowed to run python. The failure claims below come from reading the code. The two marked low confidence depend on coremltools behaviour that could not be checked.

Code Quality

  • ops_to_not_decompose does not follow tensors through getitem. backends/apple/coreml/partition/coreml_partitioner.py:549-550, 564-567

    • Problem: the loop only visits nodes whose target is an OpOverload. operator.getitem nodes never join outside, so the prediction stops at any op with several outputs (split, split_with_sizes, unbind, chunk, topk, sort).
    • Trigger: r, g, b = frame_u8.unbind(-1); return F.pad(r, (1, 1)).float() / 255.
      • aten.pad.default is predicted to stay in Core ML, so it is kept whole.
      • On the edge graph, partition() moves it to the CPU.
      • to_executorch() then fails in ToOutVarPass with Missing out variants (exir/passes/__init__.py:461-462). The message says nothing about the uint8 input.
    • Why the existing check misses it: the check after lowering (exir/program/_program.py:1418-1436) calls ops_to_not_decompose again on the edge program. Its targets are EdgeOpOverload, which is not an OpOverload subclass (exir/dialects/edge/_ops.py:291), so the call returns an empty list.
    • Second trigger: a uint8 tensor made in one Core ML partition and read by another. This happens when partitions are split to avoid a cycle through a CPU op. _find_readers_of_unsupported_inputs flags the reader (arg not in partition.nodes), but the prediction does not.
    • Fix:
      • Propagate outside through getitem.
      • Add a split/unbind test.
      • Consider raising a clear error after the loop in partition() when a node it moved is kept whole but has no out variant.

    Fix this →

  • Decomposing a uint8 reader can still leave a Core ML partition with no inputs. coreml_partitioner.py:570-573. Low confidence, not run.

    • Trigger: F.pad(frame_u8, (1, 1, 1, 1), mode="reflect"), or mode="replicate". Reflect-padding a camera frame is common.
      • aten.pad.default has no out variant, so the pad is rejected and decomposed.
      • test_uint8_made_inside_the_delegate_keeps_ops_whole shows the decomposition is an aten.index.Tensor gather, with indices built from arange.
      • The gather reads the frame, so it moves to the CPU.
      • The index computation reads no runtime input, and it cannot join the float partition after the gather without making a cycle.
    • Result: a Core ML partition with no inputs, the case the roll docstring (test file 566-568) describes. model_metadata_from_spec rejects such a partition when the deployment target is below iOS18 (backends/apple/coreml/compiler/coreml_preprocess.py:439-442).
    • Even at iOS18+: ops are rejected by target, not by node (line 580). So every float reflect pad elsewhere in the network also becomes a gather inside Core ML.
    • Possible fixes: stop the decomposition at aten.reflection_pad2d/aten.replication_pad2d, which have portable kernels (kernels/portable/functions.yaml:823, 838), or don't delegate partitions that read no runtime input. Either way, add reflect/replicate pad to the uint8 test.
  • _has_out_variant checks for an ATen .out schema, not for an ExecuTorch kernel. coreml_partitioner.py:196-201

    • Problem: to_out_variant only looks up torch's JIT schemas (exir/operator/convert.py:229-231).
    • Effect: an op that Core ML supports and that has an autogen .out but no portable or optimized kernel is kept whole on the CPU. It passes to_executorch() and then fails at load_method. A possible case is rot90, which has no entry in kernels/portable/functions.yaml (whether coremltools supports it is unverified).
    • The comment at 561-563, the test docstring at 565-566 and the PR description all treat "has an out variant" and "has a CPU kernel" as the same thing.
    • Narrow exception handling: to_out_variant can also raise AssertionError (convert.py:213-226, 308-311), or AttributeError when _get_overload_schema returns None (convert.py:303-304). Only RuntimeError is caught, and this call sits outside the try/except at 552-560. Such an op stops lowering instead of being decomposed.

    Fix this →

  • unsupported_input_readers is passed through a side channel. coreml_partitioner.py:226, 337, 475

    • Problem: partition() adds nodes to a public set on the support object between rounds. So is_node_supported answers differently for the same node depending on hidden state, and the instance in ops_to_not_decompose never sees the set.
    • Cost: each round re-runs CapabilityBasedPartitioner and coremltools' is_torch_fx_node_supported on every node, once per op along the uint8 chain.
    • Fix: pass the set in explicitly, for example as a constructor argument with a new support object each round, instead of changing it from outside the class.
  • Only partition inputs are checked, not partition outputs of these dtypes. coreml_partitioner.py:174-193. Low confidence.

    • Trigger: the loop can create such an output itself. In torch.maximum(frame_u8, (x > 0).to(torch.uint8)), moving maximum to the CPU turns the uint8 operand made inside Core ML into a delegate output.
    • Runtime side:
      • get_data_type(ScalarType) has no Char (int8) case (backends/apple/coreml/runtime/delegate/coreml_backend_delegate.mm:130-151).
      • The MLMultiArray output mapping covers only Float16/32/64 and Int32 (ETCoreMLModelManager.mm:137-155).
    • How coremltools types such outputs could not be checked here.

Backward Compatibility

  • "Models that already lowered keep the same Core ML program" is not true in general. coreml_partitioner.py:568-569
    • Problem: the prediction runs on the ATen graph and adds every node unsupported there to outside, even when the pieces it decomposes into are delegated on the edge graph.
    • Example:
      • _HIGH_RANK_LAYOUT_TARGETS (70-77) lists only edge ops, so an ATen view/permute above rank 5 is unsupported at 110-115 but delegated after decomposition.
      • Model: (x * 255).to(torch.uint8) → rank-6 reshape/permute/reshape → F.pad → .float().
      • Before this PR it lowered entirely into Core ML.
      • Now the pad reads an "outside" uint8 tensor, so aten.pad.default is rejected and every pad in the model is decomposed.
    • The trigger is contrived. At minimum, narrow the claim in the PR description.

Testing

  • test_uint8_camera_frame_is_read_outside_the_delegate (test_coreml_partitioner.py:533, 538, 556):
    • With randint(0, 100), x / 255 - 0.5 < -0.11, so relu returns all zeros. The allclose passes even with a wrong permute, conversion or delegate value.
    • A wider range alone is not enough for int8: 127/255 − 0.5 < 0 still gives all zeros, so the model has to change too.
  • Same test, 558-561: the lower_full_graph=True check runs after the loop, with the ep from the last pass. Only int8 is covered, never uint8 from CoreML partitioner errors out with 8-bit integer types #11686. Move the check into the subTest.
  • test_uint8_input_read_by_ops_with_and_without_cpu_kernels (563-596):
    • Off macOS it only checks that lowering and to_executorch() don't raise.
    • Nothing asserts that a delegate exists, that aten.roll.default stays whole, or that pad and max_pool2d were decomposed.
    • The neighbouring placement tests assert on op_names on every platform (e.g. 415-422, 512-518).
  • _placeholders_kept_inside (coreml_partitioner.py:503-516) has no test. Nothing reads a uint8/int8 parameter, buffer, lifted constant or mutable buffer inside Core ML with take_over_constant_data / take_over_mutable_buffer on and off.

Fix the tests →

Recommendation

Request Changes

  • The getitem gap is a crash in the new logic, and the fix is small.
  • The main test's end-to-end check cannot detect wrong output.
  • The reflect/replicate-pad case needs either a fix or a documented limitation.

  • Gather context (diff, commit, repo conventions)
  • Run independent reviews (3 reviewers)
  • Consolidate findings
  • Fact-check findings against the cited code
  • Post review

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@metascroy @cymbalrush @YifanShenSZ could you take a look? This changes the Core ML backend.

Copilot AI balanced review requested due to automatic review settings October 30, 2026 03:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI balanced review requested due to automatic review settings October 30, 2026 04:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI balanced review requested due to automatic review settings October 30, 2026 04:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI balanced review requested due to automatic review settings October 30, 2026 05:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI balanced review requested due to automatic review settings October 30, 2026 05:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@claude

claude Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Claude finished @shoumikhin's task in 42m 31s —— View job


This keeps ops that read uint8/int8 tensors on the CPU as intended, but at the default deployment target it can also move a state update to the CPU while a Core ML delegate keeps reading a frozen copy of that state, so the recommendation is Request Changes.

Detailed review

PR Review: #24317

Summary

The PR makes the Core ML partitioner run on the CPU two kinds of ops: those that read a uint8/int8 tensor from outside their partition, and those in any partition this leaves with no tensor input. It also makes ops_to_not_decompose decompose the non-core ops among them. The main problem is that at the default deployment target, a model with a uint8/int8 input and state now lowers without error but then computes with stale state. The other findings are smaller gaps in what the new partition boundaries allow, in the messages, and in the tests. Nothing was run, because this job can't run python or git fetch or read the coremltools/torch sources. The runtime effects below are traced from the ExecuTorch code.

Code Quality

  • Stale state at the default deployment target (major). In backends/apple/coreml/partition/coreml_partitioner.py, line 222 doesn't count kept-inside placeholders as inputs. So once the loop leaves anything out (line 251), a partition whose only outside input is a taken-over mutable buffer counts as having no inputs.

    • Without an iOS 18 minimum_deployment_target (_allows_partitions_without_inputs, 554-559), line 225 moves that partition to the CPU, while the buffer's other readers stay delegated.
    • tag_mutated_buffer ignores the untagged CPU users and gives the buffer to the reading delegate (exir/backend/utils.py:416-430). _maybe_duplicate_constant_nodes then gives the CPU ops a copy (utils.py:225-237).
    • The CPU updates its copy, and from the second call on the delegate keeps reading the initial value.
    • The model in test_uint8_input_keeps_state_in_the_delegate_from_ios18 (test file 669-679) does exactly this at the default target, which is the stale copy its docstring describes. The test only covers iOS 18, only checks op placement, and never calls to_executorch().
    • Suggested fix: don't move a partition that writes taken-over state. It is stateful, so it already needs iOS 18 (log at 536-542), and the no-input check reads the converted model's specificationVersion (backends/apple/coreml/compiler/coreml_preprocess.py:439-442), which such a model should already meet. Wherever the users of a taken-over buffer would still be split between Core ML and the CPU, raise an error that points to take_over_mutable_buffer=False or an iOS 18 target.
    • Low confidence: in the same test at iOS 18, the reader and the writer are separate delegates, and the same duplication looks like it gives the reader a frozen copy there too. Running forward twice on macOS and comparing with eager would show both cases.

    Fix this →

  • Values other than input tensors can cross the new boundaries unchecked (low confidence). The loop only checks the dtype of tensors a partition reads.

    • With a dynamic dim on the uint8 input, the sym_size that reads that input ends up on the CPU. A delegated op whose size comes from it, such as a reshape computed from image.shape[2], then reads a SymInt from outside. Its partition also reads a tensor, so line 225 keeps it. The delegate then gets the symbolic input that the docstring at 213 and the TODO at 357-372 say Core ML can't take.
    • A uint8/int8 tensor made inside a partition and read by a left-out op becomes a delegate output, for example (x > 0.5).to(torch.uint8) * image with a uint8 image. I couldn't check what coremltools does with that output dtype. The runtime's dtype map has no int8 (Char) case (backends/apple/coreml/runtime/delegate/coreml_backend_delegate.mm:130-151).
  • Skipped ops have different names in the two graphs that compute left_out (minor; the mismatch predates this PR). skip_ops_for_coreml_delegation is matched against node.target.__name__ (line 383).

    • That name is mm.default for the ATen op ops_to_not_decompose sees, but aten.mm.default for the edge op partition() sees (exir/dialects/edge/_ops.py:303). The tests pass the edge form (test file 73, 100, 433).
    • So a skipped non-core op is still preserved, then lands whole on the CPU, where it may have no kernel. Separately, the new ATen-side left_out can disagree with the one partition() computes.
    • The post-lowering check (exir/program/_program.py:1418-1436) catches neither case: ops_to_not_decompose only looks at torch._ops.OpOverload targets (621-622), so it returns nothing for edge programs.

    Fix this →

  • The new messages are hard to act on (minor).

    • The log at 523-526 joins str(node.target) once per node, and for edge ops that is the whole <EdgeOpOverload: …>: schema = … repr (exir/dialects/edge/_ops.py:334-339).
    • The lower_full_graph=True error (517-522) doesn't name the input or the dtype.
    • The log goes through CoreMLPartitioner.log_once (549-552), which uses root logging.info rather than the module logger used at 285-288, so ET_COREML_LOG_LEVEL doesn't control it.

    Fix this →

Testing

  • test_uint8_input_with_dynamic_shape (633-660) and test_uint8_buffer_not_taken_over_is_read_outside_the_delegate (697-723) only assert under if _TEST_RUNTIME:, which is false on Linux (line 45). So the Linux run checks only that lowering doesn't raise, not the op placement their docstrings describe. Fix this →
  • test_uint8_input_read_by_ops_with_and_without_cpu_kernels only lowers with the default CoreMLPartitioner() (618). At iOS 18, _allows_partitions_without_inputs() is true, so the gather index math stays in Core ML as a delegate with no inputs. The aten.arange.start_step expectations (594, 596) therefore hold only at the default target, and the iOS 18 path is never lowered or run. Fix this →

Performance

  • Extra partitioning on every export (minor). ops_to_not_decompose (613-618) now runs _propose_partitions even when the graph has no tensor of a dtype Core ML can't take.

    • Each call gets an extra CapabilityBasedPartitioner pass and a second support check per node (624).
    • to_edge_transform_and_lower calls it four times per program (exir/program/_program.py:1193, 1196→1140, 1422, 1426→1140). So a float model pays for two extra full passes over the ATen graph that yield an empty left_out.
    • The two post-lowering calls throw the result away, since edge targets fail the check at 621-622.
    • Skip the call when no node satisfies _cannot_be_coreml_input.

    Fix this →

Recommendation

Request Changes

The stale-state case gives wrong results with no error. It happens with a uint8/int8 input, state and the default deployment target, a combination that crashed with a KeyError before this PR and now lowers. The new tests would not catch it. The remaining items are minor or low confidence.

  • Gather context
  • Run independent reviews
  • Consolidate findings
  • Fact-check
  • Post review

@shoumikhin shoumikhin changed the title Keep Core ML inputs it cannot type, such as uint8, on the CPU Core ML partitioner: keep uint8 and int8 inputs on the CPU, and fix bool min and max and rejected decomposed ops Oct 30, 2026
Copilot AI balanced review requested due to automatic review settings October 30, 2026 14:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

coremltools 9.0 often ignores the dtype of an op that fills a tensor
with one value. torch.full((1, 16), 0) has no dtype, so torch makes it
int64, but coremltools makes it float. zeros and new_zeros come out
float even with an int dtype, ones_like does from iOS 16, and
full_like, fill and scalar_tensor take the type of their fill value.

An int or bool tensor made as float breaks in two ways. An op that does
not promote types, such as index_put through a bool mask, fails to
lower ("updates has dtype int32 whereas data has dtype fp32"). The
SmolVLM vision embeddings hit this. Other ops run at float16 precision
and silently lose integers above 2048.

The partitioner now refuses those fills when they make an int or bool
tensor. full, full_like, fill and scalar_tensor run on the portable
kernels. zeros, new_zeros, zeros_like, ones_like and new_full are no
longer kept whole, so lowering decomposes them into a full or full_like
with the dtype given, which Core ML handles. Float fills are unchanged.
With lower_full_graph=True, a model with such a fill now raises instead
of lowering with the wrong dtype.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 01:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@shoumikhin shoumikhin changed the title Core ML partitioner: keep uint8 and int8 inputs on the CPU, and fix bool min and max and rejected decomposed ops Core ML partitioner: keep uint8 and int8 inputs on the CPU, and fix bool min and max, int and bool fills, and rejected decomposed ops Oct 31, 2026
@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@claude

claude Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

PR Review: #24317

  • Gather context (diff, callers in exir/, Core ML runtime)
  • Run three independent reviews
  • Consolidate findings (34 candidates → 14)
  • Fact-check each finding against the code (11 kept, 3 dropped)
  • Post review

View job run

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@claude

claude Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

PR Review: #24317

  • Gather context (diff, commit history, repo conventions) — the checkout is shallow, so the diff is taken against the merge base 681e493; this job cannot run python, and the torch and coremltools sources are not installed
  • Run three independent reviews
  • Consolidate findings (26 candidates)
  • Fact-check each finding against the code
  • Post review

View job run

torch.nn.RMSNorm with its default eps=None exports rms_norm without the
eps argument. coremltools 9.0 requires that argument, so lowering fails
with "node aten_rms_norm_default (rms_norm) got 3 input(s), expected
[4]", and it turns a None eps into 1e-5. Torch computes RMSNorm in
float32 and uses the float32 machine epsilon, about 1.19e-7, also for
float16 and bfloat16 inputs.

The Core ML backend now gives coremltools that eps when the model leaves
it out, so float32 models with such an RMSNorm lower and match torch. A
float16 model still fails in coremltools' rms_norm, with or without eps.
The branch for an older coremltools, which has no rms_norm conversion,
is unchanged.
torch.export passes the approximate argument of gelu as a keyword. The
coremltools 9.0 converter reads it only from the positional inputs, so
gelu(approximate="tanh") silently becomes the exact gelu. The two
differ by up to 4.7e-4, at float32 and float16 precision alike, which
looks like poor float32 accuracy in Core ML.

The Core ML backend now registers a gelu conversion that also reads the
keyword. A tanh gelu written out with pow was not affected, since
coremltools fuses that formula into the right mode.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 23:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@shoumikhin shoumikhin changed the title Core ML partitioner: keep uint8 and int8 inputs on the CPU, and fix bool min and max, int and bool fills, and rejected decomposed ops Core ML: keep uint8 and int8 inputs on the CPU, and fix bool min and max, fills, tanh GELU, RMSNorm and rejected decomposed ops Oct 31, 2026
coremltools converts ops such as clone into nothing, so a Core ML
partition can return one of its own inputs, as in
"return x.clone(), x * 2", or in "y = x.clone()" right before an op that
runs on the CPU. It can also return one value twice, as in
"return y, y.clone()". When the partition has other outputs, Core ML
hands back such an output wrong: all zeros, or the bytes of another
output.

The Core ML backend now runs one more coremltools pass, which gives
each output that repeats an input or an earlier output an identity op of
its own, so every output comes from an op and has its own name.

The clone test now returns such outputs. It checks them at run time,
and checks the pass on the converted program, which needs no runtime.
When a constant has users in different partitions, or in a partition
and on the CPU, to_backend gives each user its own copy of the
placeholder. duplicate_constant_node only replaced the constant in the
top-level args and kwargs of a user, so an op that reads it from a list,
such as cat or the indices of index_put, kept the original. Lowering
then failed with "constant data node ... is tagged with ... but has user
... which has tag (None)". For example, a Core ML model whose index
buffer is read by a gather in Core ML and by an index_put on the CPU
failed this way.

The pass now replaces the constant wherever the user reads it.
coremltools 9.0 converts index_put with a whole slice, such as
y[:, idx] = v, by writing v into the part of y the indices select, with
each indexed dim kept as size 1. That only works with accumulate=False,
int indices of shape () or (1,), a static size in every other dim, a v
that broadcasts to that part, and a non-bool y below iOS 18. With a 0-d
index or several indices, torch lays out that part differently, so v
must not vary before the last index, or the values land in the wrong
place. With two entries in idx, as in the X-VLA policy, lowering fails
with "Input squeeze_0 has rank 1 != other inputs rank 0". Without a
whole slice, it fails when several index tensors have different shapes,
as in y[rows[:, None], cols] = v, or include a bool mask.

The partitioner now refuses those forms, so they run on the portable
kernels. The forms coremltools handles stay in Core ML, such as a
single-entry index next to a slice for a cache update, a scalar written
through several single-entry indices, or index tensors of one shape
without a slice.

A negative index next to a slice still stays in Core ML, since the check
cannot see index values: it fails, or gives wrong values when it comes
from an input.

The fill test now checks how many calls of an op stay on the CPU, since
its index_put model has several forms that must all be refused.
The tanh gelu and RMSNorm tests and the int zeros case of the fill test
only failed without their fixes when the Core ML runtime ran the
program, so they passed on runners without it. The gelu test now also
checks that the converted program uses the tanh mode, the RMSNorm test
checks the eps in the converted program, and the fill test checks that
the int zeros is not kept whole, so lowering decomposes it.

The RMSNorm override now also says why it has to stay once coremltools
accepts a missing eps: coremltools would use 1e-5, not the torch eps.

The RMSNorm test is skipped in fbcode, whose coremltools has no rms_norm
converter to check.
to_edge_transform_and_lower asks the partitioner twice in a row for the
ops to keep whole, with the same program. When the program has an op
Core ML rejects that has a decomposition, such as an int zeros or a
bfloat16 linear, each answer decomposes the whole program in a dry run,
which is slow for a large model.

The Core ML partitioner now keeps its last answer and returns it again
for the same program, as the MLX partitioner does. It holds the program
through a weak reference, so it never keeps a program alive.
The partitioner raised for every state buffer whose readers and update
were in different places. But tag_mutated_buffer only moves a buffer
into Core ML when an op in a partition reads it. When only CPU ops read
it, for example an op Core ML rejects or skips, the buffer stays a
program buffer and the delegate returns its new value, which worked
before the check. Such a model, like

    y = torch.acosh(self.state)
    self.state.copy_(x * 2)

failed to lower with "used in more than one place".

The check now skips a buffer that no partition reads.
coremltools makes full without a dtype, zeros, new_zeros and, from iOS
16, ones_like as float, zeros_like with the dtype of its input, and
full_like, new_full, fill and scalar_tensor with the type of their fill
value, instead of with the dtype torch gives the result. So an int or
bool tensor came out as float: index math on it failed to lower, or lost
integers above 2048 at float16 precision. A float tensor could come out
as int or bool too: the zeros_like(mask, dtype=float) that
nn.MultiheadAttention makes from a bool mask stayed bool, and attention
came out wrong. The partitioner refused the int and bool cases, which
moved them to the CPU, and with lower_full_graph=True it failed models
that worked before, such as x + torch.full((1, 16), 0).

The backend now converts these ops with the dtype in the node's
metadata, and the partitioner no longer refuses them. A TorchScript
node, which has no such metadata, and a fill value known only at run
time keep the coremltools converter.
The partitioner refuses some ops that coremltools converts, such as an
index_put form coremltools gets wrong, or an op on a bfloat16 tensor.
With lower_full_graph=True, lowering then failed with "CoreML does not
support the op index_put" and advice to ask coremltools for the op,
although coremltools has it. The real reason only went to the info log.

Lowering now fails with the reason, for example "aten.index_put.default
with these indices is not supported by CoreML. Set
lower_full_graph=False in the CoreMLPartitioner to run it outside of
CoreML." The info log keeps the same reasons, now with op names instead
of their schemas.
The dry run in ops_to_not_decompose drops kept ops that end up on the
CPU, and runs again, since a dropped op gets decomposed and can change
the program. A dropped op with no decomposition, such as the cast of a
uint8 input, changes nothing, so the next round repeated the same
decomposition and search only to find nothing more to drop. The dry run
now stops there, and builds the decomposition table once.

Its docstring also says that it does not see transform passes, or the
ops an earlier partitioner takes.
When ops in more than one place use a buffer that Core ML takes over,
lowering fails, since each place would use its own copy. The error said
only that the buffer is used in more than one place. For a model whose
cache update runs on the CPU, such as an index_put Core ML cannot
convert, it did not say which op that is.

The error now lists each op that uses the buffer and where it runs, and
the docs for take_over_mutable_buffer mention the error.
Copilot AI balanced review requested due to automatic review settings November 1, 2026 12:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@shoumikhin shoumikhin changed the title Core ML: keep uint8 and int8 inputs on the CPU, and fix bool min and max, fills, tanh GELU, RMSNorm and rejected decomposed ops Core ML: run ops that read uint8 and int8 inputs on the CPU, refuse ops coremltools gets wrong, and fix fills, tanh GELU, RMSNorm and repeated outputs Nov 1, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: apple Changes to the Apple backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants