Skip to content

examples/llama: fix FSDP grad-accum-fusion regression + propagate train exit status#145

Merged
sudhu2k merged 2 commits into
rocm_devfrom
fix/fsdp-grad-accum-fusion
Jul 7, 2026
Merged

examples/llama: fix FSDP grad-accum-fusion regression + propagate train exit status#145
sudhu2k merged 2 commits into
rocm_devfrom
fix/fsdp-grad-accum-fusion

Conversation

@wenchenvincent

Copy link
Copy Markdown
Collaborator

Summary

Two independent fixes to examples/llama/train_llama{2,3}.sh:

  1. Disable gradient accumulation fusion on FSDP paths. The perf toggle added in examples/llama: default-on fusion & DP-comm perf toggles #141 defaults GRADIENT_ACCUMULATION_FUSION on, but fusion is incompatible with both FSDP backends, so every FSDP run (the test harness drives --fsdp 1) broke:

    • torch-FSDP2 (FSDP=1 FR --use-torch-fsdp2) hard-asserts in arguments.py: --use-torch-fsdp2 is not suu pported with gradient accumulation fusion.
    • Megatron-FSDP (MEGATRON_FSDP=1) crashes at runtime: [FSDP][Rank N][fsdp_grads] No buffer found for bucket_id ....

    Fix: force GRADIENT_ACCUMULATION_FUSION=0 inside the existing FSDP || MEGATRON_FSDP block (with a notice). The non-FSDP path keeps fusion on.

  2. Propagate the training exit status. The scripts exited 0 even when torchrun crashed, so CI gating on exit code saw failures as passes ~@~T which is how the examples/llama: default-on fusion & DP-comm perf toggles #141 regression slipped through green. Two masks: eval $run_cmd stt
    atus was discarded (and with TEE_OUTPUT=1 the ... |& tee pipeline returns tee's status, ~0), and the trailing perf-parse became the script's exit code. Fix: set -o pipefail + capture $?, emit an explicit error line, and exit $TRAIN_RC at the end.

Verification (8~WMI355X, MODEL_SIZE=8, mock data, 3 iters)

Config Before After
FSDP2 + fusion on assert trains (fusion auto-off)
Megatron-FSDP + fusion on grad-bucket crash trains (fusion auto-off)
no-FSDP + fusion on ok fusion stays on
forced torchrun failure exit 0 (masked) exit != 0 (TEE_OUTPUT 0 and 1)
clean run exit 0 exit 0

Immediate unblock without the script change: run with GRADIENT_ACCUMULATION_FUSION=0.

wenchenvincent and others added 2 commits June 30, 2026 19:21
The fusion perf toggle defaults ON, but gradient accumulation fusion is
incompatible with both FSDP backends: torch-FSDP2 hard-asserts against it in
arguments.py (--use-torch-fsdp2 is not supported with gradient accumulation
fusion), and Megatron-FSDP crashes at runtime with an fsdp_grads "No buffer
found for bucket_id" assertion. Since the test harness drives FSDP=1, the
default-on toggle broke every FSDP run.

Force GRADIENT_ACCUMULATION_FUSION=0 inside the existing FSDP/MEGATRON_FSDP
block (with a notice). The non-FSDP perf path keeps fusion on. Verified on
8xMI355X (MODEL_SIZE=8, mock data): FSDP=1 and MEGATRON_FSDP=1 both train,
non-FSDP keeps fusion enabled.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
train_llama{2,3}.sh exited 0 even when torchrun crashed, so CI gating on exit
code saw failures as passes. Two masks: (1) the run status was discarded, and
with TEE_OUTPUT=1 the `... |& tee $TRAIN_LOG` pipeline returns tee's status
(~always 0), not torchrun's; (2) the trailing throughput/mem perf-parse became
the script's exit status.

Wrap the run in `set -o pipefail` and capture $? (eval collapses PIPESTATUS
to a single element = tee's, so PIPESTATUS[0] does not work here), emit an
explicit error line on failure, and `exit $TRAIN_RC` at the end so the
perf-parse still runs but cannot mask a crash. Verified: clean run exits 0;
forced torchrun failure exits non-zero under both TEE_OUTPUT=0 and =1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wenchenvincent
wenchenvincent requested a review from sudhu2k June 30, 2026 20:50
@sudhu2k

sudhu2k commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Megatron-FSDP should support Gradient accumulation fusion. The error that you saw was a bug from the upstream which was fixed recently in the commit: https://github.com/NVIDIA/Megatron-LM/pull/5222/changes

Basically, the error occurs only when you use double buffers for ping-ponging, when you disable it, it runs fine for me with gradient_accumulation_fusion.

[2026-06-30 21:40:55.941706] iteration 9/ 12 | consumed samples: 2304 | elapsed time per iteration (ms): 32108.9 | mem usages: 0.6143 | throughput per GPU (TFLOP/s/GPU): 420.2 | learning rate: 1.000000E-04 | global batch size: 256 | lm loss: 3.268351E-02 | loss scale: 1.0 | grad norm: 0.202 | num zeros: 0 | number of skipped iterations: 0 | number of nan iterations: 0 |

The issue happens because of overflow of requirement for inflight buffers when we only use 2 buffers for AG and RS.
When both the buffers are being used (One of AG and one for compute), RS might be scheduled which would require another buffer and this would overflow the fixed size of 2.
The fix has some variables which doesn't exist in our current fork, but I've added a work-around fix in the following PR #146

For now, I think it's fine to disable MegatronFSDP when used with gradient accumulation fusion, I can enable this path again in my PR.

@sudhu2k sudhu2k left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let me know what your thoughts are on my previous comment.

@wenchenvincent

Copy link
Copy Markdown
Collaborator Author

@sudhu2k Let's get it disabled for now to unblock. Then once the fix is in, we can reenable it.

@sudhu2k sudhu2k left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Will re-enable Gradient accumulation with MegatronFSDP in another PR.
#146

@sudhu2k
sudhu2k merged commit 6caffff into rocm_dev Jul 7, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants