Skip to content

fix ple grads under sp - #234

Merged
hjh0119 merged 2 commits into
modelscope:mainfrom
hjh0119:fix/ple-sp-tp-grad
Oct 15, 2026
Merged

hjh0119 merged 2 commits into
modelscope:mainfrom
hjh0119:fix/ple-sp-tp-grad

Conversation

@hjh0119

@hjh0119 hjh0119 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

fix #233

PLE's SP forward gathers the full sequence, computes and scatters the output
back, so every TP rank already produces the complete full-sequence gradient for
the replicated key_proj/value_proj/norm_*/conv1d parameters. Marking them
sequence_parallel=True made finalize_model_grads SUM those complete gradients
again across TP, scaling the updates (and global grad clipping) by tp_size --
a silent gradient correctness bug (issue modelscope#233); TP=2/TP=8 regressions show 2x/8x
vs the full-sequence reference before the fix.

- ple.py: sync the replicas via average_gradients_across_tp_domain, matching
  the olmoe q/k-norm precedent.
- tuners/lora.py: a base whose own weight is AVG-synced computes on the
  gathered sequence, so the replicated LoRA factors wrapped around it (e.g.
  PLE's projections) get the same AVG sync instead of the SP sum.
- tests/test_ple_sp_tp_grad.py: real NCCL TP ranks and the real
  finalize_model_grads, compared against the TP=1 full-sequence reference,
  with a negative control that reproduces the tp_size amplification.
@hjh0119
hjh0119 merged commit 8b567b3 into modelscope:main Oct 15, 2026
1 check passed
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.

[Bug] Qwen3.8 PLE gradients are incorrectly scaled by TP size when sequence parallelism is enabled

2 participants