Conversation
FP16_Optimizer.step()/step_fused_adam() and FP16_UnfusedOptimizer.step()/ step_fused_lamb() call logger.info() directly for grad-overflow and dynamic-loss-scale-change messages, so every rank prints the same line on every occurrence instead of only rank 0. On a many-node run this floods logs with hundreds of duplicate lines, drowning real signal. Use the existing log_dist(..., ranks=[0]) helper instead, matching the precedent already used for the sibling "Overflow detected" message in FP16_Optimizer.step() (added in deepspeedai#416) and the rank-gated messages in the ZeRO path's DynamicLossScaler.update_scale(). Fixes deepspeedai#1533 Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Fixes #1533
Problem
FP16_Optimizer(fused_optimizer.py) andFP16_UnfusedOptimizer(
unfused_optimizer.py) calllogger.info()directly for grad-overflow anddynamic-loss-scale-change messages inside
step(),step_fused_adam()/step_fused_lamb(), and_update_scale(). Every rank prints the same line onevery occurrence instead of only rank 0, so on a many-node run these flood the
logs with hundreds of duplicate lines and drown out real signal, exactly as
reported.
Impact: contract-violation. Who reaches it / triggered by: any multi-GPU or
multi-node training run that enables fp16 with dynamic loss scaling (the
default
fp16config) and hits a grad overflow or a scale change — whichhappens routinely during normal fp16 training, not just on error. What is
observed: every rank prints the identical "Grad overflow on iteration",
"Reducing/Increasing dynamic loss scale", and "fp16 dynamic loss scale
overflow! Skipping step" lines, instead of only rank 0. This repo already
established rank-0-only logging as the correct behavior for this exact
message class — the sibling "Overflow detected" message in
FP16_Optimizer.step()was fixed this way in #416, and the ZeRO path'sDynamicLossScaler.update_scale()gates every one of its own overflowmessages on
dist.get_rank() == 0— so this PR just closes the same gap inthe two files the issue names.
Fix
Route the remaining messages through the existing
log_dist(..., ranks=[0])helper instead of
logger.info(), matching the precedent above.Testing
Added
test_overflow_logs_only_on_rank_zero(parametrized over bothoptimizer classes) in
tests/unit/runtime/half_precision/test_dynamic_loss_scale.py. It builds aminimal CPU optimizer, forces an overflow with a NaN gradient, and fakes
dist.get_rank()to simulate rank 0 vs. rank 1 (no GPU or real multi-processjob needed to observe this logging behavior).
Negative control (revert only the two production files to
upstream/master,keep the new test): fails without the fix —
Restored:
2 passed. Fulltest_dynamic_loss_scale.py: unchanged pre-existingresults before/after this diff (2 passed here plus 3 pre-existing
environment failures needing a build toolchain this sandbox lacks —
identical on unmodified
upstream/master).yapf/flake8/check-torchdist.py/check-license.pyclean on all changedfiles.
Signed-off-by: Udaya Tejas udayatejas2004@gmail.com