Repository navigation
Fail fast on non-finite AutoQuantize output gradients - #2432
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 13b0bd65abdf4b7d418fb5dce375dc5ee9914ac3 and e6eca8e. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughAutoQuantize now rejects non-finite output gradients before sensitivity accumulation. Tests verify error handling and restoration of model, scoring, and attention backend state. ChangesAutoQuantize gradient validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change has no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2432 +/- ##
==========================================
+ Coverage 71.50% 78.97% +7.47%
==========================================
Files 590 590
Lines 64749 64750 +1
==========================================
+ Hits 46297 51136 +4839
+ Misses 18452 13614 -4838
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (gpt-5.6-sol) — DM the bot to share feedback.
LGTM: the fail-fast check is scoped correctly, restores session state, preserves backend settings, and has focused non-finite-gradient coverage.
Needs action:
- Provide human sign-off for the justified edit to
test_backward_scoring_session_restores_partial_setup, which adds coverage that failure cleanup leaves cuDNN SDPA unchanged.
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (gpt-5.6-sol) — DM the bot to share feedback.
Approved: the fail-fast check is scoped correctly, preserves scoring cleanup, and has focused NaN and infinity coverage.
No action needed:
- ✔️ The prior sign-off request is no longer applicable:
test_backward_scoring_session_restores_partial_setupremains unchanged; this PR only inserts new tests before it.
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
13b0bd6 to
e6eca8e
Compare
What does this PR do?
Type of change: Bug fix
Fail fast when AutoQuantize receives non-finite output gradients, before accumulating sensitivity scores. The error names the affected module and suggests checking the model, data, and loss; it also identifies cuDNN SDPA backward on fully masked rows as one possible cause and gives an explicit retry workaround.
Unlike the earlier revision, this does not disable cuDNN or change any attention backend settings. Invalid gradients are not zeroed or ignored, and there is no automatic retry.
Usage
No API or recipe changes. For the reproduced cuDNN failure, the caller can explicitly set
torch.backends.cuda.enable_cudnn_sdp(False)before a fresh AutoQuantize run.Testing
8025a3dc5481129aa21fef99cb13a879e1b5847eplus this patch, batch size 8, 512 calibration samples, andw4a16_nvfp4_fp8_at_6p0bits-active_moe.yaml:model.language_model.layers.39.self_attn.q_proj; cuDNN remained enabled and no quantized model was exported. Expected-error check passed.git diff --checkpassed.Before your PR is "Ready for review"
Contributor guidelines and security guidance followed; commits are signed and signed off.
CONTRIBUTING.md: N/A; neither added.Additional Information
This improves error reporting rather than fixing the upstream cuDNN kernel. Blackwell-specificity is not established. Checkpoint deployment/reload was not tested.
A calibration-only checkpoint-resume attempt completed scoring but encountered a separate
candidate_statsKeyError. That issue is outside this patch; the successful export validation above used a fresh run without search-state resume.Summary by CodeRabbit