Conversation
Resubmitting the batched-gradient half of ndif-team#699 on its own, per the review there. Nothing currently asserts this: invokes share one batched forward, so a module's output is a single tensor holding every invoke's rows, and reading `.grad` inside an invoke has to narrow to that invoke's slice. Reading the whole batch, or another invoke's slice, yields a plausible-looking tensor that is silently the wrong gradient. Four cases, against `reference_grad` — what plain autograd says for that input alone: * Parametrised over which invoke takes the gradient, first through last of three. Each sits at a different batch offset, so a slice taken from the wrong place is only caught by exercising all of them. * Row counts differ per invoke (2 / 3 / 1), so a wrong offset fails on shape rather than on values that might round close. * A same-row-count pair, where the shape check cannot be what catches a wrong offset — only the values can — asserting the gradient read is *not* the other invoke's. * Two invokes each taking a gradient, and an edit in one leaving the other's rows untouched. Confirmed these have teeth with a mutant that forces the gradient to be read from batch row 0: geometry = (tensor.shape, tensor.stride(), 0) Five of the six new tests fail against it — every one whose target is not row 0, including the same-row-count pair, which catches it on values alone. All seven pre-existing tests in the file still pass under the mutant, which is the gap this fills. `_BatchEnvoy` is duplicated from `test_batching.py` rather than imported across test modules; that file is deliberately all `@torch.no_grad()`, and these are the gradient tests. Happy to hoist it to `conftest.py` instead if you would rather have one copy. Tests only — no source changes.
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.
The batched-gradient half of #699, on its own, per your review there:
No diagnostic, no docs — those are settled. Tests only, no source changes.
What it pins
Invokes share one batched forward, so a module's output is a single tensor holding every invoke's rows. Reading
.gradinside an invoke has to narrow to that invoke's slice; reading the whole batch, or another invoke's slice, gives a plausible-looking tensor that is silently the wrong gradient.Four cases, all against
reference_grad— what plain autograd says for that input alone:Confirming they have teeth
Since nothing asserted this before, I checked against a mutant that forces the gradient to be read from batch row 0:
Every new test whose target is not row 0 fails — including the same-row-count pair, which catches it on values alone. All seven pre-existing tests in the file still pass under the mutant, which is the gap this fills.
One judgement call
_BatchEnvoyis duplicated fromtest_batching.pyrather than imported across test modules — that file is deliberately all@torch.no_grad(), and these are the gradient tests. Happy to hoist it toconftest.pyinstead if you would rather have one copy; I left it local rather than touching a shared file unasked.Verification
tests/test_backward.py: 13 passed (7 existing + 6 new).0.8@ fe8645c, Python 3.14.3,transformers5.16.1,torch2.9.1, CPU.