Skip to content

Declare gradient accumulation loss scaling with loss_is_scaled_for_ga - #7508

Draft
qgallouedec wants to merge 3 commits into
mainfrom
loss-is-scaled-for-ga
Draft

qgallouedec wants to merge 3 commits into
mainfrom
loss-is-scaled-for-ga

Conversation

@qgallouedec

Copy link
Copy Markdown
Member

Uses Trainer.loss_is_scaled_for_ga (huggingface/transformers#49240) to say whether compute_loss already scales for gradient accumulation, instead of steering it through model_accepts_loss_kwargs and compute_loss_func.

  • loss_is_scaled_for_ga = False (Trainer divides by GA steps): DPO, KTO, RLOO, Reward, CPO, ORPO. Replaces self.model_accepts_loss_kwargs = False.
  • loss_is_scaled_for_ga = True (trainer normalizes over the accumulated batch itself): GRPO, Distillation, AsyncGRPO, AsyncDistillation, SDFT, SDPO, SSD. Replaces compute_loss_func="non-None value to disable scaling" plus self.model_accepts_loss_kwargs = False.
  • SFT keeps the default None.

For transformers < 5.19, _BaseTrainer.__init__ maps the flag back to the two attributes, so behaviour there is unchanged. That block goes away when the floor reaches 5.19.

Step-1 grad_norm for DPO, Reward and GRPO, batch 4 x GA 1 and batch 2 x GA 2: identical to main on transformers 5.18 and on transformers with #49240.

Merge after huggingface/transformers#49240.

This branch has not been deployed

No deployments
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.

1 participant