Skip to content

Commit a07997b

Browse files
committed
Numba: allow broadcast scatter in IndexedElemwise
Allow the written buffer in IndexedElemwise to be larger than scalar in the core loop. When the Elemwise computes a scalar per index iteration and the target has extra non-indexed dims, the scalar broadcasts into those dims via o[...] = scalar. Key changes: - Generalize _getitem_0d_ellipsis to any ndim so o[...] += scalar works for 1-d+ core output arrays - Relax the broadcast guard to allow val with fewer non-indexed dims when the indexed axes are rightmost (no trailing non-indexed dims) - Transpose the target buffer in the fusion rewrite when non-indexed dims precede the indexed axis, so core dims are trailing (gufunc convention)
1 parent 1ab7f4c commit a07997b

4 files changed

Lines changed: 151 additions & 35 deletions

File tree

pytensor/link/numba/dispatch/elemwise.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ def impl(*outer_inputs):
489489

490490
return impl
491491

492-
cache_version = (0, 4)
492+
cache_version = (0, 5)
493493
if scalar_cache_key is None:
494494
key = None
495495
else:

pytensor/link/numba/dispatch/vectorize_codegen.py

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,23 @@
2121
from pytensor.link.numba.dispatch import basic as numba_basic
2222

2323

24-
# Numba is missing getitem(0d_array, Ellipsis), so o[...] += val fails.
24+
# Numba is missing getitem(array, Ellipsis), so o[...] += val fails.
2525
# Register it so store_core_outputs can use o[...] += val naturally.
2626
@overload(operator.getitem, inline="always")
2727
def _getitem_0d_ellipsis(arr, idx):
28-
if (
29-
isinstance(arr, types.Array)
30-
and arr.ndim == 0
31-
and isinstance(idx, types.EllipsisType)
32-
):
28+
if isinstance(arr, types.Array) and isinstance(idx, types.EllipsisType):
29+
if arr.ndim == 0:
30+
31+
def impl(arr, idx):
32+
return arr[()]
33+
34+
return impl
35+
else:
3336

34-
def impl(arr, idx):
35-
return arr[()]
37+
def impl(arr, idx):
38+
return arr
3639

37-
return impl
40+
return impl
3841

3942

4043
def encode_literals(literals: Sequence) -> str:

pytensor/tensor/rewriting/indexed_elemwise.py

Lines changed: 100 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -438,23 +438,51 @@ def find_indexed_update_consumers(fgraph, node):
438438
if info is None:
439439
continue
440440
target, idx_axis_pairs, mode = info
441-
# Don't fuse if the value broadcasts on the index loop dim
442-
# (constant across index — recomputing per position is wasteful)
443-
# or against non-indexed target axes.
441+
# Don't fuse if val is broadcastable on any index loop dim
442+
# the Elemwise result is constant across the index and
443+
# recomputing it per index position is wasteful.
444444
val = client_node.inputs[1]
445445
n_idx_dims = max((idx.ndim for idx, _ in idx_axis_pairs), default=0)
446446
val_idx_bc = list(val.type.broadcastable)[:n_idx_dims]
447447
if any(val_idx_bc):
448448
continue
449+
450+
# Check broadcast compatibility between val and target on
451+
# non-indexed axes. Val may have fewer non-indexed dims
452+
# than target — the extra target dims become core dims
453+
# (broadcast output) — but ONLY when the indexed axes are
454+
# rightmost in the target so that val's dims align with the
455+
# index loop (numpy broadcasts from the right). When
456+
# indexed axes are NOT rightmost, val's dims would align
457+
# with trailing non-indexed axes instead, giving wrong
458+
# results if fused.
449459
indexed_axes = {a for _, a in idx_axis_pairs}
460+
max_indexed_axis = max(indexed_axes)
461+
has_trailing_non_indexed = any(
462+
a > max_indexed_axis
463+
for a in range(target.type.ndim)
464+
if a not in indexed_axes
465+
)
450466
non_indexed_target_bc = [
451467
bc
452468
for i, bc in enumerate(target.type.broadcastable)
453469
if i not in indexed_axes
454470
]
455471
non_indexed_val_bc = list(val.type.broadcastable)
472+
n_idx_dims = max((idx.ndim for idx, _ in idx_axis_pairs), default=0)
456473
non_indexed_val_bc = non_indexed_val_bc[n_idx_dims:]
457-
if len(non_indexed_val_bc) < len(non_indexed_target_bc) or any(
474+
if len(non_indexed_val_bc) < len(non_indexed_target_bc):
475+
# Val has fewer dims — broadcasting into extra target
476+
# dims. Only safe when indexed axes are rightmost
477+
# (no trailing non-indexed dims).
478+
# TODO: could also handle trailing non-indexed dims
479+
# that are broadcastable (size-1) in the val — squeeze
480+
# them away before fusing. Requires excluding
481+
# local_AdvancedIncSubtensor_to_AdvancedIncSubtensor1
482+
# to encounter these graphs naturally.
483+
if has_trailing_non_indexed:
484+
continue
485+
elif any(
458486
vbc and not tbc
459487
for vbc, tbc in zip(
460488
non_indexed_val_bc, non_indexed_target_bc, strict=False
@@ -593,15 +621,50 @@ def find_indexed_update_consumers(fgraph, node):
593621
dup_map = {}
594622

595623
# Inner fgraph outputs; add update targets
624+
# For non-axis-0 indexed updates, transpose the target so the
625+
# indexed axis moves to position 0 and core dims are trailing
626+
# (matching the gufunc convention assumed by the codegen).
596627
inner_outputs = list(node.outputs)
597628
call_inputs = list(inner_inputs)
629+
scatter_transpose_back = {} # scatter_idx -> inverse perm
598630
for out_idx in sorted(update_consumers.keys()):
599631
update_node, target, idx_axis_pairs, _mode = update_consumers[out_idx]
600632

633+
# Check if we need to transpose the target
634+
max_idx_axis = max(a for _, a in idx_axis_pairs)
635+
needs_transpose = any(
636+
a < max_idx_axis
637+
for a in range(target.type.ndim)
638+
if a not in {a for _, a in idx_axis_pairs}
639+
)
640+
if needs_transpose:
641+
# Build permutation: indexed axes first, then rest
642+
idx_axes_sorted = sorted({a for _, a in idx_axis_pairs})
643+
non_idx_axes = [
644+
a for a in range(target.type.ndim) if a not in idx_axes_sorted
645+
]
646+
perm = idx_axes_sorted + non_idx_axes
647+
inv_perm = [0] * len(perm)
648+
for i, p in enumerate(perm):
649+
inv_perm[p] = i
650+
target = target.dimshuffle(perm)
651+
# Remap idx_axis_pairs to the transposed axes
652+
axis_remap = {old: new for new, old in enumerate(perm)}
653+
idx_axis_pairs = [
654+
(idx_var, axis_remap[axis]) for idx_var, axis in idx_axis_pairs
655+
]
656+
# Update the consumers entry with remapped axes
657+
update_consumers[out_idx] = (
658+
update_node,
659+
target,
660+
idx_axis_pairs,
661+
_mode,
662+
)
663+
601664
inner_inputs.append(target)
602665

603666
target_pos = len(call_inputs)
604-
if update_node.op.inplace:
667+
if update_node.op.inplace and not needs_transpose:
605668
call_inputs.append(target)
606669
else:
607670
call_inputs.append(target.copy())
@@ -611,9 +674,20 @@ def find_indexed_update_consumers(fgraph, node):
611674
# The value to scatter: use the (possibly duplicated) Elemwise output
612675
scatter_value = node.outputs[scatter_idx]
613676

614-
# Build the scatter output using the correct value
615-
if update_node.op.inplace:
616-
# Rebuild with the new value
677+
# Build the scatter output
678+
# After transpose, indexed axes are at position 0+,
679+
# so always use AdvancedIncSubtensor1 for single axis-0.
680+
if len(idx_axis_pairs) == 1 and idx_axis_pairs[0][1] == 0:
681+
inplace_op = AdvancedIncSubtensor1(
682+
inplace=True,
683+
set_instead_of_inc=update_node.op.set_instead_of_inc,
684+
)
685+
scatter_out = inplace_op(
686+
target,
687+
scatter_value,
688+
idx_axis_pairs[0][0],
689+
)
690+
elif update_node.op.inplace and not needs_transpose:
617691
if isinstance(update_node.op, AdvancedIncSubtensor1):
618692
scatter_out = update_node.op(
619693
target, scatter_value, update_node.inputs[2]
@@ -622,14 +696,6 @@ def find_indexed_update_consumers(fgraph, node):
622696
scatter_out = update_node.op(
623697
target, scatter_value, *update_node.inputs[2:]
624698
)
625-
elif isinstance(update_node.op, AdvancedIncSubtensor1):
626-
inplace_op = AdvancedIncSubtensor1(
627-
inplace=True,
628-
set_instead_of_inc=update_node.op.set_instead_of_inc,
629-
)
630-
scatter_out = inplace_op(
631-
target, scatter_value, update_node.inputs[2]
632-
)
633699
else:
634700
inplace_op = AdvancedIncSubtensor(
635701
idx_list=update_node.op.idx_list,
@@ -642,6 +708,9 @@ def find_indexed_update_consumers(fgraph, node):
642708
inner_outputs[scatter_idx] = scatter_out
643709
outer_destroy_map[scatter_idx] = [target_pos]
644710

711+
if needs_transpose:
712+
scatter_transpose_back[scatter_idx] = inv_perm
713+
645714
# Build indexed_inputs spec for the Op
646715
indexed_inputs_spec = [None] * n_indices
647716
for idx_var, _ax, positions in read_groups:
@@ -660,13 +729,21 @@ def find_indexed_update_consumers(fgraph, node):
660729
indexed_inputs_spec[k] = ((), ax, iv.type.broadcastable)
661730
break
662731

663-
# Build indexed_outputs spec for the Op
732+
# Build indexed_outputs spec for the Op.
733+
# Use the (possibly remapped) axis from update_consumers.
664734
indexed_outputs_spec = [None] * n_indices
665735
for out_idx in sorted(update_consumers.keys()):
666736
_update_node, _target, idx_axis_pairs, mode = update_consumers[out_idx]
667737
for idx_var, axis in idx_axis_pairs:
668-
key = (idx_var, axis)
669-
idx_pos = all_idx_groups[key][1]
738+
# Look up in all_idx_groups using the ORIGINAL key
739+
# (all_idx_groups was built before any transpose).
740+
idx_pos = None
741+
for (iv, _), (v, p) in all_idx_groups.items():
742+
if iv is idx_var and p is not None:
743+
idx_pos = p
744+
break
745+
if idx_pos is None:
746+
continue
670747
scatter_idx = dup_map.get(out_idx, out_idx)
671748
if indexed_outputs_spec[idx_pos] is None:
672749
indexed_outputs_spec[idx_pos] = ([scatter_idx], mode, axis)
@@ -685,6 +762,10 @@ def find_indexed_update_consumers(fgraph, node):
685762
indexed_outputs=indexed_outputs_spec,
686763
)(*call_inputs, return_list=True)
687764

765+
# Transpose back any scatter outputs that were transposed
766+
for scatter_idx, inv_perm in scatter_transpose_back.items():
767+
new_outs[scatter_idx] = new_outs[scatter_idx].dimshuffle(inv_perm)
768+
688769
# The node whose outputs we need to replace in the outer graph
689770
orig_node = old_node if multi_client_outs else node
690771
replacements = []

tests/link/numba/test_indexed_elemwise.py

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -269,10 +269,13 @@ def test_negative_indices(self):
269269
class TestIndexedUpdateFusion:
270270
"""Test indexed updates (AdvancedIncSubtensor1) fused into Elemwise."""
271271

272-
def test_no_fusion_when_val_broadcasts_against_target(self):
273-
"""Don't fuse (yet) if elemwise output broadcasts against target's trailing axes.
272+
def test_no_fusion_when_idx_axes_outside_elemwise_loop(self):
273+
"""Don't fuse if the indexed axes are not within the Elemwise loop.
274274
275-
TODO: support this by making the update output's core_ndim > 0.
275+
Here the index is on axis 0 of target(5, 10), but the Elemwise
276+
output (10,) corresponds to axis 1 (the non-indexed trailing axis).
277+
The indexed axis doesn't overlap with the Elemwise computation, so
278+
fusing would misalign which input dims map to which target dims.
276279
"""
277280
rng = np.random.default_rng(42)
278281
idx = rng.integers(5, size=10).astype(np.int64)
@@ -282,9 +285,9 @@ def test_no_fusion_when_val_broadcasts_against_target(self):
282285
elemwise_out = advanced_subtensor1(x, idx) + y # shape (10,)
283286
out = pt.inc_subtensor(target[idx], elemwise_out)
284287
fn, fn_u = fused_and_unfused([x, y, target], out)
285-
# Write not fused — val (10,) would broadcast to (10, 10) in the target.
286-
# Read fusion still creates an IndexedElemwise, but the
287-
# AdvancedIncSubtensor1 must remain outside it.
288+
# Write not fused — the Elemwise loop dim is the non-indexed axis,
289+
# not the indexed axis. Read fusion may still create an
290+
# IndexedElemwise, but the AdvancedIncSubtensor1 must remain outside.
288291
assert any(
289292
isinstance(n.op, AdvancedIncSubtensor1) for n in fn.maker.fgraph.toposort()
290293
)
@@ -314,6 +317,35 @@ def test_no_fusion_when_val_broadcasts_along_index_dim(self):
314317
tv = rng.normal(size=(5,))
315318
np.testing.assert_allclose(fn(xv, tv), fn_u(xv, tv), rtol=1e-10)
316319

320+
def test_broadcast_val_into_non_indexed_dims(self):
321+
"""Fuse when Elemwise output broadcasts into target's non-indexed dims.
322+
323+
target(5, 3)[:, idx] += exp(val) — the Elemwise loop covers the
324+
index axis (axis 1), and the scalar result broadcasts into the
325+
non-indexed leading axis (axis 0, core_ndim=1).
326+
327+
Requires excluding local_AdvancedIncSubtensor_to_AdvancedIncSubtensor1
328+
so we keep AdvancedIncSubtensor on the correct axis.
329+
"""
330+
rng = np.random.default_rng(42)
331+
idx = rng.integers(3, size=10).astype(np.int64)
332+
target = pt.matrix("target", shape=(5, 3))
333+
val = pt.vector("val", shape=(10,))
334+
out = pt.inc_subtensor(target[:, idx], pt.exp(val))
335+
336+
mode = NUMBA_MODE.excluding(
337+
"local_AdvancedIncSubtensor_to_AdvancedIncSubtensor1"
338+
)
339+
mode_u = NUMBA_NO_FUSION.excluding(
340+
"local_AdvancedIncSubtensor_to_AdvancedIncSubtensor1"
341+
)
342+
fn = pytensor.function([val, target], out, mode=mode, trust_input=True)
343+
fn_u = pytensor.function([val, target], out, mode=mode_u, trust_input=True)
344+
assert_fused(fn)
345+
valv = rng.normal(size=(10,))
346+
tv = rng.normal(size=(5, 3))
347+
np.testing.assert_allclose(fn(valv, tv), fn_u(valv, tv), rtol=1e-10)
348+
317349
def test_inc_subtensor(self):
318350
rng = np.random.default_rng(42)
319351
idx = rng.integers(85, size=919).astype(np.int64)

0 commit comments

Comments
 (0)