Repository navigation
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. |
…racked NVIDIA#6965 stopped passing is_first_microbatch to TE when the config does not maintain it. The predicate it added reads the config alone, so it answers True for every module as soon as quant_recipe is set. A recipe decides quantization per module and in both directions: it can quantize a layer the config leaves alone, and it can force a layer to high precision while the config enables fp8. A layer of the second kind has no quantized weight cache, so the flag is never refreshed for it, and TE derives the weight-gradient accumulation mode from that same argument -- accumulate = fuse_wgrad_accumulation and not is_first_microbatch -- so a stale True makes its wgrad GEMM overwrite main_grad instead of accumulating into it. Give is_first_microbatch_tracked optional module and is_context_quantized arguments and consult will_execute_quantized when a module is in hand, which is the same hook fp8_utils and the MoE experts already use. Callers with no module, namely MegatronModule.set_is_first_microbatch, keep the model-wide answer. Kitchen returns early because it drives quantization outside TE's autocast state, so there is no per-module answer to consult. Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
deepakn94
force-pushed
the
dnarayanan/is_first_microbatch_per_module
branch
from
September 15, 2026 17:41
a4fc390 to
ac37391
Compare
jaredcasper
approved these changes
Sep 15, 2026
zhongbozhu
requested changes
Sep 17, 2026
zhongbozhu
left a comment
Contributor
There was a problem hiding this comment.
Do we still need this PR if we have this #6965 merged already?
This branch was previously deployed
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?
Follows up #6965. That PR stopped passing
is_first_microbatchto TE when the config does not maintain it, at config granularity. This extends the same predicate to per-module granularity, because aquant_recipedecides quantization per module and in both directions.Not a bug fix
Stating this up front, since the phrasing in an earlier revision of this description overstated it. A mis-tracked module today still runs once per microbatch, and
set_is_first_microbatchstill re-arms the flag every step, because it asks the config and the config does name a recipe. So the microbatch-1Truelands on a gradient buffer that was just zeroed, and nothing is lost.This removes a trap rather than repairing an observed failure. The staleness that motivated #6965, where nothing re-arms the flag at all, does not apply here.
The gap
is_first_microbatch_trackedreads the config alone:It answers
Truefor every module as soon asquant_recipeis set, including a module the recipe forces to high precision while the config enables fp8.TEQuantizationRecipeleaves bothfp8_quantization_recipeandfp4_quantization_recipeunset for that case, and_get_should_context_be_quantized_recipereturnsFalsefor it.Such a module has no quantized weight cache to refresh, so the flag carries no information for it. It is not inert, though: TE selects the weight-gradient accumulation mode from the same argument,
so the module takes the overwrite path for nothing. That is survivable only because the buffer is freshly zeroed and the module is invoked once. It becomes real gradient loss for any module invoked more than once per microbatch, which is exactly the failure #6965 fixed for repeated MTP layers.
Testing
te_quant_params is not Nonewould not be enough either: a force-high-precision entry is still a recipe match, so the params are present while the layer runs in high precision. The question has to be answered from the recipe contents.The change
Delegate to TE's own answer.
will_execute_quantizedreads the recipe contents rather than its presence, honoursoverride_quantized_autocast, and distinguishes the training and evaluation recipes. It is the same hookfp8_utils.py:965and the MoE experts already use, so no new mechanism is introduced._resolve_is_first_microbatchpasses the module and the ambient autocast state, read before entering the module's own quantization context, since that is the state a recipe overrides.MegatronModule.set_is_first_microbatch, keep the model-wide answer unchanged. Verified across all sixteen combinations offp8,fp4,use_kitchenandquant_recipe.use_kitchenreturns early: Kitchen drives quantization outside TE's autocast state, so there is no per-module answer to consult.Behaviour is unchanged for any config without a
quant_recipe.Tests
Seven cases in
tests/unit_tests/transformer/test_module.py, nine once parametrization expands, none needing TE or a GPU: an unquantized config; the model-wide answers with no module passed, which pin whatset_is_first_microbatchrelies on; Kitchen staying tracked even when the module says otherwise; a recipe quantizing an otherwise-unquantized layer; a recipe forcing high precision under fp8 being untracked; a module with no recipe following the ambient autocast in both directions; and a module lacking the hook falling back to the config.tests/unit_tests/determinism/kernels/test_te_wrappers.pygains two cases for_resolve_is_first_microbatchon a real TE linear, sincemegatron/core/extensions/transformer_engine.pyis a registered kernel source.Pre-checks
black, isort, pylint (10.00/10) and ruff clean. The unit tests were not executed locally: the
megatron.coreimport chain does not load on macOS, and the determinism cases need a GPU with TE.🤖 Generated with Claude Code