Fix ZeroDivisionError in fused linear cross entropy when weight vocab dim is 0 - #1353
Open
nileshpatil6 wants to merge 2 commits into
Open
Fix ZeroDivisionError in fused linear cross entropy when weight vocab dim is 0#1353nileshpatil6 wants to merge 2 commits into
nileshpatil6 wants to merge 2 commits into
Conversation
… dim is 0 When weight has vocab dimension 0, for example when accessing a DeepSpeed ZeRO-3 partitioned parameter directly before it is gathered, inc_factor in fused_linear_cross_entropy_forward becomes 0 and the subsequent triton.cdiv(BT, inc_factor) call raises a bare ZeroDivisionError deep inside triton with no context about what went wrong. This adds an assertion that fails early with a message explaining the likely cause, and adds a regression test that reproduces the original crash and confirms the new error message. Fixes linkedin#767
Author
|
Giving this one a bump since it's been quiet since the resubmit. It's a small guard, one ZeroDivisionError fix with a test, rebased on current main and mergeable. CI never ran on this reopened version, so if a maintainer can approve the workflow run I'll take care of anything it flags right away. |
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.
Summary
Fixes #767. This reopens #1289, which I accidentally closed for good by deleting my fork during a repo cleanup (GitHub won't let a PR reopen once the head repo is gone). Same fix, rebased onto current main.
fused_linear_cross_entropy_forwardcomputes chunking as:When
weighthas a vocab dim of 0,inc_factoris 0 and the nextcdivdivides by zero. The user just seesZeroDivisionError: integer division or modulo by zeroraised from inside triton with nothing pointing at the real cause. That is what the reporter in #767 hit, and it comes up with DeepSpeed ZeRO-3 when a partitioned parameter is touched directly before it has been gathered.This adds an assert before the chunking math with a message that names the actual problem and what to do about it, instead of letting it fall through to triton.
Details
Kept it as an assert to match the existing validation style at the top of the same function (
return_z_loss,return_token_accuracy,return_predicted_tokensall assert). Happy to switch it to aValueErrorif you would rather it not be strippable under-O.Testing Done
Added
test_empty_weight_raises_clear_errorintest/transformers/test_fused_linear_cross_entropy.py. It runs on CPU since the assert fires before any kernel launch, so it does not need a GPU.I do not have a GPU on this machine, so I want to be upfront about what I could and could not verify:
New test passes, and I confirmed it actually catches the bug: reverting just the source change makes it fail with
ZeroDivisionError: integer division or modulo by zerofromtriton/__init__.py, and restoring the fix makes it pass.No regressions: unmodified main gives 141 failed / 0 passed on this file, and with my change it is 141 failed / 1 passed. Same failures either way, all of them
RuntimeError: 0 active driversfrom triton because there is no CUDA device here. The only difference is my new test passing.ruff checkandruff format --checkboth clean on the two changed files.Could not run
make testormake test-convergencefor real, both need a GPU. Would appreciate CI covering those.Hardware Type: CPU only (no GPU available, see note above)
run
make testto ensure correctnessrun
make checkstyleto ensure code stylerun
make test-convergenceto ensure convergence