Skip to content

Fix MLX Reshape: resolve the shape at funcify time - #2395

Open
guillaume-osmo wants to merge 1 commit into
pymc-devs:mainfrom
guillaume-osmo:fix-mlx-reshape
Open

Fix MLX Reshape: resolve the shape at funcify time#2395
guillaume-osmo wants to merge 1 commit into
pymc-devs:mainfrom
guillaume-osmo:fix-mlx-reshape

Conversation

@guillaume-osmo

Copy link
Copy Markdown

Closes #2386.

Motivation

mlx_funcify_Reshape forwarded the shape input straight to mx.reshape, but the linker typifies every input to mx.array and mx.reshape only accepts a Python sequence of ints, so every reshape raised TypeError.

Reading the array back at runtime is not an option either: the linker enables mx.compile by default and MLX forbids evaluating a traced array, so the shape has to be resolved when the dispatch is built.

Implementation

Resolve the target shape at funcify time, preferring the statically inferred output shape:

  1. node.outputs[0].type.shape, when fully known. This already resolves any -1, and it also covers shapes read off another input (x.reshape(y.shape)), which are not Constants but which PyTensor's shape inference does resolve.
  2. Otherwise a Constant shape input.
  3. Otherwise a runtime conversion, which raises NotImplementedError with an explanation instead of a bare eval error from inside MLX. This is the case the existing test_mlx_Reshape_shape_graph_input xfail already describes.

Mirrors the approach in pytensor/link/jax/dispatch/shape.py.

Tests

Four tests in tests/link/mlx/test_shape.py were already red on main and are green now:

  • test_mlx_Reshape_constant
  • test_mlx_Reshape_various_shapes
  • test_mlx_Reshape_negative_one
  • test_mlx_Reshape_concrete_shape

Two added, both under the full "MLX" mode (where the typification actually happens):

  • test_mlx_Reshape_full_mlx_mode — constant, -1, and 1-d/2-d/3-d targets
  • test_mlx_Reshape_shape_from_other_inputx.reshape(y.shape)

Full tests/link/mlx/ suite: no regressions.

`mlx_funcify_Reshape` forwarded the shape input straight to `mx.reshape`,
but the linker typifies every input to `mx.array` while `mx.reshape` only
accepts a Python sequence of ints, so every reshape raised `TypeError`.

Reading the array back at runtime is not an option either: the linker
enables `mx.compile` by default and MLX forbids evaluating a traced array,
so the shape has to be resolved when the dispatch is built. Prefer the
statically inferred output shape (which already resolves any `-1`, and
covers shapes read off another input such as `x.reshape(y.shape)`), then
fall back to a constant shape input. A genuinely data-dependent shape now
raises `NotImplementedError` with an explanation instead of an eval error
from deep inside MLX.

This turns four already-red tests in `tests/link/mlx/test_shape.py` green
and adds regression tests under the full "MLX" mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MLX backend: every reshape raises TypeError (shape passed as mx.array)

1 participant