apple / apple/coremltools

TorchConverter: states=[] cannot express "this model has no states"

Open Beginner friendly
#2,826 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
5.4k
Forks
850
Avg merge
4d 5h
Merged PRs (30d)
10

Description

## Summary

`ct.convert(..., states=[])` and `ct.convert(...)` are indistinguishable inside
`TorchConverter`, so a caller converting an `ExportedProgram` has no way to say "do not
turn my mutable buffers into states". The empty list is normalised into the same value as
the default before the decision is made.

`coremltools/converters/mil/frontend/torch/converter.py`, in `TorchConverter.__init__`
(9.0, lines 579-582):

```python
# process states
if states is None:
states = []
self.states = states
```

and then, on the `ExportedProgram` path (line 640):

```python
if states is None or len(states) == 0:
# For torch.export, we default to create states from torch mutable buffers
self.states = []
for name, tensor in self.graph.buffers.items():
...
self.states.append(state)
```

By the time that runs, `states is None` can never be true — it was rebound to `[]` above —
so the condition reduces to `len(states) == 0`, and an explicit empty list takes the
inference branch. Every mutable buffer becomes a state whatever the caller asked for.

## Why it matters

ExecuTorch's Core ML backend has a `take_over_mutable_buffer=False` option, and cannot
honour it. Its converter call has no way to express the intent, and the models it produces
then require an `MLState` the runtime was not built to supply, failing at execute rather
than at conversion (pytorch/executorch#21855). The comment in the ExecuTorch source says
the option exists precisely because the OSS runtime does not handle state.

## Suggested change

Keep the caller's intent before normalising:

```python
# process states
# An empty list is the caller saying the model has no states, which is not the
# same as saying nothing; only the latter asks for them to be inferred.
infer_states_from_buffers = states is None
if states is None:
states = []
self.states = states
```

and use `infer_states_from_buffers` in place of `states is None or len(states) == 0`.

Callers passing `None` or a non-empty list are unaffected. The one behaviour that changes
is `states=[]`, which today means "infer them" and would come to mean what it reads as.

## What I did not do

I could not reduce the ExecuTorch symptom below a full LLM export, so this report is from
reading the conversion path rather than from a minimal end-to-end repro. The two code
paths above are quoted verbatim from the installed 9.0 source, and the ExecuTorch side is
in pytorch/executorch#21855 with its own detail.

Environment: coremltools 9.0, torch 2.13.0, macOS arm64.

Contributor guide

Open the contributing guide

Research direction

Read coremltools/converters/mil/frontend/torch/converter.py, especially TorchConverter.__init__ and the ExportedProgram path around lines 579-582 and 640. Preserve whether states was omitted before normalising it, then verify that omitted states still infer mutable-buffer states while states=[] leaves the model without states; add or run the relevant converter regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
machine-learning
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.