TorchConverter: states=[] cannot express "this model has no states"
- 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
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