Fix AdvancedIncSubtensor runtime-broadcast check crashing on a scalar update - #2339
Fix AdvancedIncSubtensor runtime-broadcast check crashing on a scalar update#2339velochy wants to merge 1 commit into
AdvancedIncSubtensor runtime-broadcast check crashing on a scalar update#2339Conversation
|
Separate from this fix, while verifying that the guard still fires I found that the numba backend Same graph, a length-1 import numpy as np, pytensor, pytensor.tensor as pt
from pytensor.compile.mode import Mode
x, y = pt.vector("x"), pt.vector("y")
out = pt.set_subtensor(x[pt.constant([1, 3])], y)
for label, mode in [
("py ", Mode(linker="py", optimizer=None)),
("cvm/C ", Mode(linker="cvm", optimizer="fast_run")),
("numba ", Mode(linker="numba", optimizer="fast_run")),
("jax ", "JAX"),
]:
try:
print(label, pytensor.function([x, y], out, mode=mode)(np.zeros(5), np.ones(1)))
except Exception as e:
print(label, f"{type(e).__name__}: {str(e).splitlines()[0]}")So Worth being explicit that this is not a missing call that could be dropped in next to the others. Given that, it seemed like your call rather than something to fold into a crash fix. Happy to open One caveat on my own numbers: |
|
@ricardoV94 I know I am maxing out my PR quota again, but this one seems small and contained... hopefully :) |
|
May sound pedantic but that AdvancedIncSubtensor is expecting a 0d array type, not a scalar, so the bug is actually elsewhere, a loose typify or another op. >>> import numpy as np, pytensor, pytensor.tensor as pt
...
... x = pt.vector("x")
... out = pt.set_subtensor(x[pt.constant([1, 3])], 1.0) # scalar update
... out.dprint(print_type=True)
...
AdvancedSetSubtensor [id A] <Vector(float64, shape=(?,))>
├─ x [id B] <Vector(float64, shape=(?,))>
├─ 1.0 [id C] <Scalar(float32, shape=())>
└─ [1 3] [id D] <Vector(int64, shape=(2,))>That scalar is a 0d float32 array. Should have a shape. But JAX typify defaults to "everything is fine", that's where the bug is. |
|
You're right, and it isn't pedantic — I was patching the symptom. The input is a 0d @jax_typify.register(np.ndarray)
def jax_typify_ndarray(data, dtype=None, **kwargs):
if len(data.shape) == 0:
return data.item() # 0d array -> Python scalar
return jnp.array(data, dtype=dtype)
But dropping the
The new ones cluster where JAX needs a static Python value rather than an array — 10 in How would you like to play it? Options as I see them:
Happy with whichever; I have the reproducer and the suite runs set up either way. For context on why |
|
3 sounds the best bandaid |
set_subtensor(x[idx_vector], 1.0) raises AttributeError: 'float' object has no attribute 'shape' under the JAX backend. The update input is a 0d TensorType, so _check_runtime_broadcast_of_vector_index is entitled to a value with a .shape, but jax_typify downgrades 0d arrays to Python scalars before the op ever sees them. Coerce y back to an array at the dispatch, which is the layer that knows it is handing linker values to a check written against arrays. The .item() in jax_typify is load-bearing for dispatches that need a static Python scalar (axis/shape/loop-bound arguments), so it is left alone.
2b2474f to
fa85b07
Compare
|
Done — narrowed to option 3. Happy to open a follow-up issue for the typify part if you want it tracked. |
What / why
set_subtensor(x[idx_vector], scalar)works on the Python and numba backends and raises on JAX:The update input is a 0d
TensorType, so_check_runtime_broadcast_of_vector_indexis entitled toa value with a
.shape. It doesn't get one becausejax_typifydowngrades 0d arrays to Pythonscalars:
The change
Coerce at the dispatch call site — the layer that knows it is handing linker values to a check
written against arrays:
This is a bandaid over the typify behaviour, by @ricardoV94's call in the thread below. Removing the
.item()branch instead does fix the crash on its own, but it takestests/link/jax/from 2failures to 27 (25 new, verified against the same suite on clean
main) — 10 intest_scan.py,4 in
test_sort.py(ValueError: Non-hashable static arguments), 3 intest_shape.py, pluslinalg/pad/conv, mostlyTracerBoolConversionErrorand non-hashable static args. The.item()is load-bearing for dispatches that want a static Python value for an axis, shape or loopbound, so doing it properly means having those pull their scalar from the constant node instead —
a much wider change than this PR.
Testing
test_jax_AdvancedIncSubtensor1_scalar_y, parametrised overadvanced_inc_subtensor1/advanced_set_subtensor1to match the neighbouringtest_jax_AdvancedIncSubtensor1_runtime_broadcast. Verified it fails without the fix(
AttributeError) and passes with it.test_jax_AdvancedIncSubtensor1_runtime_broadcaststill passes: a length-1ywritten into 2 indices continues to raise
ValueError: Runtime broadcasting not allowed…, so theguard still fires where it should.
tests/link/jax/test_subtensor.pypasses (11 passed, 1 xfailed).tests/tensor/test_subtensor.pyhas one failure,
TestSubtensor::test_boolean, which fails identically on the base commit.Multinomialover best/worst choice sets) whoseexpansion function builds its rank structure with a scalar
set_subtensor. Undernuts_sampler="nutpie"with the JAX backend this surfaced only as an opaqueError during point expansionpanic from nuts-rs mid-sampling, with the Python exceptionswallowed. Three multi-step MRP models (~5k respondents) that could not be fitted under the JAX
backend at all now complete; the numba backend fit all three identically before and after.
Versions: pytensor
main@a85bc78, jax 0.6.2, numpy 2.3.5, Python 3.12, Linux.Related: #2241 extended this check to the MLX backend and #2303 fixed a different false positive in
it; neither covers the 0d-
ycase. The MLX and PyTorch dispatches call the same method, so they arepresumably reachable the same way if their typify does the same thing — not touched here, since I
have not reproduced it on either.