Skip to content

Fix #45305 + add regression test GAS#45349

Merged
SunMarc merged 11 commits intohuggingface:mainfrom
florian6973:fix-gas-leak
Apr 13, 2026
Merged

Fix #45305 + add regression test GAS#45349
SunMarc merged 11 commits intohuggingface:mainfrom
florian6973:fix-gas-leak

Conversation

@florian6973
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #45305
Add a regression test in TrainerGradientAccumulationTest to avoid passing the GAS value to Accelerate by mistake

Description: I force the value of the num_steps parameter to be 1, and the regression test just checks after the Trainer creation that the num_steps value is 1 and not something else.

Code Agent Policy

The Transformers repo is currently being overwhelmed by a large number of PRs and issue comments written by
code agents. We are currently bottlenecked by our ability to review and respond to them. As a result,
we ask that new users do not submit pure code agent PRs at this time.
You may use code agents in drafting or to help you diagnose issues. We'd also ask autonomous "OpenClaw"-like agents
not to open any PRs or issues for the moment.

PRs that appear to be fully agent-written will probably be closed without review, and we may block users who do this
repeatedly or maliciously.

This is a rapidly-evolving situation that's causing significant shockwaves in the open-source community. As a result,
this policy is likely to be updated regularly in the near future. For more information, please read [CONTRIBUTING.md](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md).

  • I confirm that this is not a pure code agent PR.

Before submitting

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of who to tag.
Please tag fewer than 3 people.

Research projects are not maintained and should be taken as is.

Copy link
Copy Markdown
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

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

Thanks !! just a couple of comments

Comment thread src/transformers/trainer.py Outdated
Comment thread tests/trainer/test_trainer.py Outdated
Comment thread tests/trainer/test_trainer.py Outdated
Comment thread tests/trainer/test_trainer.py Outdated
Comment thread tests/trainer/test_trainer.py Outdated
Comment thread tests/trainer/test_trainer.py Outdated
@florian6973
Copy link
Copy Markdown
Contributor Author

Just addressed your comments, please let me know if you have any other feedback. Thank you!

Copy link
Copy Markdown
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

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

Thanks a lot for iterating !

@SunMarc SunMarc enabled auto-merge April 13, 2026 13:50
@SunMarc SunMarc added this pull request to the merge queue Apr 13, 2026
@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 13, 2026
@SunMarc SunMarc added this pull request to the merge queue Apr 13, 2026
Merged via the queue into huggingface:main with commit 20b6f53 Apr 13, 2026
28 checks passed
@vasqu vasqu added the for patch Tag issues / labels that should be included in the next patch label Apr 13, 2026
ArthurZucker pushed a commit that referenced this pull request Apr 13, 2026
* Fix #45305 + add regression test GAS

* Refine test model_accepts_loss_kwargs

* fix style

* Fix properly setup model_accepts_loss_kwargs+True

* Update tests/trainer/test_trainer.py

remove unnecessary parameters

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>

* fix: simplify error messages, back to a simpler test

* feat: add new test with actual training

---------

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
sirzechs66 pushed a commit to sirzechs66/transformers that referenced this pull request Apr 18, 2026
* Fix huggingface#45305 + add regression test GAS

* Refine test model_accepts_loss_kwargs

* fix style

* Fix properly setup model_accepts_loss_kwargs+True

* Update tests/trainer/test_trainer.py

remove unnecessary parameters

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>

* fix: simplify error messages, back to a simpler test

* feat: add new test with actual training

---------

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for patch Tag issues / labels that should be included in the next patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gradients not averaged by GAS when using DeepSpeed + model_accepts_loss_kwargs=True (Qwen3, Llama3, etc.)

4 participants