Skip to content

Guard zero-gradient blocks in the default FP8 block-scaling recipe - #3651

Closed
HollowMan6 wants to merge 2 commits into
NVIDIA:mainfrom
HollowMan6:bwd_grad_amax
Closed

HollowMan6 wants to merge 2 commits into
NVIDIA:mainfrom
HollowMan6:bwd_grad_amax

Conversation

@HollowMan6

Copy link
Copy Markdown
Contributor

Description

Set the default backward-gradient amax_epsilon in Float8BlockScaling to 1e-12. This gives zero-amax E5M2 gradient blocks a nonzero floor in the recipe, instead of requiring downstream projects to override the recipe instance. Forward input and weight quantization defaults remain unchanged.

Fixes https://github.com/NVIDIA/Megatron-LM/pull/7523/changes#r4211643974

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

  • Use amax_epsilon=1e-12 for the default blockwise FP8 backward-gradient quantizer.
  • Document the backward-only default.
  • Test zero-gradient quantization and Linear backward, and compare ordinary nonzero-block quantization bitwise with the previous epsilon-zero setting.

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Set the default backward-gradient `amax_epsilon` in `Float8BlockScaling` to
`1e-12`. This gives zero-amax E5M2 gradient blocks a nonzero floor in the
recipe, instead of requiring downstream projects to override the recipe
instance. Forward input and weight quantization defaults remain unchanged.

- Use `amax_epsilon=1e-12` for the default blockwise FP8 backward-gradient
  quantizer.
- Document the backward-only default.
- Test zero-gradient quantization and Linear backward, and compare ordinary
  nonzero-block quantization bitwise with the previous epsilon-zero setting.
- The focused TE GPU test passed on each of four H20 nodes using the patched
  installed Python recipe.
- Both power-of-two and FP32 scale modes passed focused tests.
- Megatron-LM's TP=2 blockwise FP8 determinism test passed after removing its
  recipe override.

Signed-off-by: Hollow Man <hollowman@opensuse.org>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 11:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added the community-contribution PRs from external contributor outside the core maintainers, representing community-driven work. label Oct 8, 2026
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Changes default FP8 quantization epsilon for backward gradients.

The PR appears safe to merge, with the previous test gap addressed.

What we checked:

  • Expected scales match the kernel: The kernel floors the block maximum to epsilon, divides the FP8 maximum by it, and rounds down to a power of two when requested. This matches the test. The 128-by-128 input leaves no padded scale entries.

Summary

Sets the default backward-gradient amax_epsilon in Float8BlockScaling to 1e-12. Forward defaults stay unchanged.

  • Adds recipe-default and zero-gradient backward checks.
  • Strengthens zero-block tests to check the expected inverse scales in both directions.
  • No new actionable issues found.

Reviews (2) · Last reviewed commit: "address review" · Reviewed by Greptile

Signed-off-by: Hollow Man <hollowman@opensuse.org>
@ptrendx

ptrendx commented Oct 9, 2026

Copy link
Copy Markdown
Member

Not fully against merging this PR, but this sounds more like the problem with the quantization method to be honest - having amax equal to 0 should be perfectly fine, we just should guard against creating the infinite scale from it (and instead have some predefined scale in that case, like 1 or something).

@ptrendx ptrendx self-assigned this Oct 10, 2026
@HollowMan6

Copy link
Copy Markdown
Contributor Author

Thank you @ptrendx for your suggestions, please find discussions at NVIDIA/Megatron-LM#7523 (comment), for that I will convert this into draft and let you know if the changes are really necessary here.

@HollowMan6
HollowMan6 marked this pull request as draft October 10, 2026 08:17
@HollowMan6

Copy link
Copy Markdown
Contributor Author

Closing as it seems this defensive change is no longer needed.

@HollowMan6 HollowMan6 closed this Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution PRs from external contributor outside the core maintainers, representing community-driven work.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants