Fix pad with an axes subset and negative axes - #4364
Draft
kapellirohith wants to merge 1 commit into
Draft
Conversation
edge_pad ignored its axes argument and indexed the pad-size vectors by array axis, reading past their end when padding a subset of the axes and placing the input on the wrong axis when axes was permuted. reflect_pad indexed by position but never normalized a negative axis. Axis handling in the pad family now goes through normalize_axis_index, which also range checks, replacing the hand-rolled normalization in pad() that indexed out_shape with an unvalidated axis.
kapellirohith
force-pushed
the
edge-pad-axes
branch
from
August 20, 2026 17:13
060d24c to
011eb45
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
edge_padignores itsaxesargument and indexes the pad-size vectors byarray axis rather than by position in
axes(ops.cpp:1550, 1556). Padding asubset of the axes therefore reads past the end of those vectors, and a
permuted
axeslist silently pads the wrong axis:A debug build traps in
SmallVector::operator[]withedge_padatops.cpp:1551 on the stack. ASan is quiet at small
ndimbecauseShapeis aSmallVectorwith 10 inline slots, so the read stays inside the object. Oncendimputs the pad sizes on the heap it reports the overflow directly:reflect_padindexes by position and is correct for a subset, but it does notnormalize a negative axis (ops.cpp:1468, 1476), so
axes = {-1}indexesstarts[-1]:Axis handling across the pad family now goes through
normalize_axis_index,the same consolidation #4288 applied to
split,unstack,partitionandtopk. That helper also range checks, which changes behaviour in one place:padpreviously indexedout_shapewith an unvalidated axis (ops.cpp:1621),so an out-of-range axis was undefined behaviour for every mode. It now raises
std::invalid_argument. That is a new error path, not just a bug fix.padvalidates and normalizes every axis before the mode dispatch, so thecalls inside
edge_padandreflect_padare redundant by construction.Normalizing once in
padand passing the normalized vector down measures+27/-20 on this file against +22/-18, and it changes what the
Padprimitivestores, and with it
is_equivalent,state()and the axesPad::vmapsees.Keeping the helper at each site leaves the primitive untouched, but the
single-call version is a small change if you prefer it.
edge_padlanded in 635ccd9 (2024-08-06, #1309).reflect_padis newer,9f35f77 (2026-08-11, #3608), and already indexes by position; this adds the
axis normalization it was missing and brings
edge_padin line with it.constantwas already correct in both respects.This is reachable from
mlx_padin mlx-c (mlx/c/ops.h:730), which forwardscaller-supplied
axes, independent pad-size lengths, and amodestring. NoPython path reaches it: the bindings only build full-length
axes(python/src/ops.cpp:3493), which is also why the existing tests miss it, the
only C++ test on this overload using a 1-D array where
axes.size() == ndim.Checklist
Put an
xin the boxes that apply.pre-commit run --all-filesto format my code / installed pre-commit prior to committing changes