Skip to content

fix gdn sp - #253

Merged
hjh0119 merged 1 commit into
modelscope:mainfrom
hjh0119:fix/gdn-sp-projection-params
Nov 3, 2026
Merged

hjh0119 merged 1 commit into
modelscope:mainfrom
hjh0119:fix/gdn-sp-projection-params

Conversation

@hjh0119

@hjh0119 hjh0119 commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@hjh0119
hjh0119 merged commit d9c371f into modelscope:main Nov 3, 2026
1 check passed
hjh0119 pushed a commit that referenced this pull request Nov 4, 2026
…254)

#253 makes `_force_sequence_parallel_off` clear `sequence_parallel` on the
projections and on their parameters. Without the parameter half, the gradient
finalizer still sums the projection gradient over the tensor parallel group even
though every rank already holds the complete copy, which is the #241 report (a
4x gamma gradient at TP4).

The new test drives the real `_force_sequence_parallel_off` on a Transformer
Engine shaped stand-in and reads the reduction back off `main_grad` after
Megatron's own `_allreduce_non_tensor_model_parallel_grads`:

* every projection gets the flag cleared on both the submodule and the marked
  parameters, and unmarked parameters are left alone;
* a `linear_decoupled_in_proj` build (no `in_proj`, no `out_proj`) does not
  raise;
* `out_norm` keeps its flag -- its weight is shared by all heads while the head
  axis is TP-split, so that gradient really is partial and still needs the sum;
* on two gloo ranks, the gamma gradient stays at 1x with the flag cleared and
  comes out at tp_size when the flag is put back, so the test fails if the
  parameter half of the toggle goes missing.

CPU only (gloo), no GPU and no NCCL.
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