Fix pad_input_tensors to pad the requested dim, not always dim 0 - #4150
Open
shoemoney wants to merge 1 commit into
Open
Fix pad_input_tensors to pad the requested dim, not always dim 0#4150shoemoney wants to merge 1 commit into
shoemoney wants to merge 1 commit into
Conversation
_pad_input_tensors resized new_size[0] unconditionally while slicing on the caller-supplied dim, so any dim != 0 raised a RuntimeError on the tensor assignment. Resize new_size[dim] instead, and port the bounds-check + negative-dim normalization already used by the sibling _pad_across_processes so out-of-range/negative dim values are handled the same way in both functions. Adds a regression test covering dim=1 padding.
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.
What does this PR do?
pad_input_tensorsaccepts a documenteddimparameter, but the inner_pad_input_tensorsalways resizednew_size[0]while slicing ondim:Any call with
dim != 0raises at the assignment:The fix resizes
new_size[dim]instead ofnew_size[0]. I also ported thebounds-check and negative-dim normalization that the sibling
_pad_across_processes(~30 lines above) already has:I'm treating this as in-scope rather than scope creep: once
dimisactually used to index
new_size, an out-of-range or negativedimneedsthe same guard
_pad_across_processesalready applies, or it produces anIndexError/ silently-wrongnew_sizeinstead of the RuntimeError above.It's the same "correctly handle
dim" concern, not an unrelated addition,and it keeps the two sibling pad functions behaving consistently for
invalid
dimvalues.Not fixed here
The
to_paddivisibility arithmetic a few lines above this change (theremainder/last_inputs/to_padblock) is a separate, pre-existingbug and is already covered by #4093. This PR does not touch it — the diff
here is scoped to
dimhandling only.Testing
Added
test_slice_and_concatenate_dimtotests/test_utils.py, next tothe existing
test_slice_and_concatenate. Confirmed it fails onmainand passes with the fix:
Before the fix:
After the fix:
Full
tests/test_utils.py:ruff checkandruff format --checkon the touched files both passclean (
All checks passed!/2 files already formatted).Before submitting
Who can review?
@SunMarc — core parts of the library.