Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix redundant checkpointing in example training scripts #33131

Merged
merged 16 commits into from
Aug 27, 2024
Merged

fix redundant checkpointing in example training scripts #33131

merged 16 commits into from
Aug 27, 2024

Conversation

eminorhan
Copy link
Contributor

What does this PR do?

Fixes #32653.

Briefly, in several of the example training scripts, running with gradient_accumulation_steps > 1 currently causes gradient_accumulation_steps times redundant checkpointing in the step-based checkpointing mode. This PR fixes the issue by adding a clause to the step-based checkpointing condition to make sure saving is done only once at the appropriate checkpointing step.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@ArthurZucker

@eminorhan eminorhan changed the title fix redundant checkpointing in example scripts fix redundant checkpointing in example training scripts Aug 26, 2024
@eminorhan eminorhan mentioned this pull request Aug 26, 2024
Closed
4 tasks
LysandreJik
LysandreJik approved these changes Aug 27, 2024
View reviewed changes
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Thanks for your PR @eminorhan!

Just a quick ping @SunMarc as you work with the Trainer and example scripts much more than I do and might see something I don't

SunMarc
SunMarc approved these changes Aug 27, 2024
View reviewed changes
Copy link
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 for finding and fixing the issue @eminorhan ! I left a suggestion to propagate to other scripts if you think it is better !

examples/pytorch/image-classification/run_image_classification_no_trainer.py Outdated
@@ -544,7 +544,7 @@ def collate_fn(examples):
completed_steps += 1

if isinstance(checkpointing_steps, int):
if completed_steps % checkpointing_steps == 0:
if completed_steps % checkpointing_steps == 0 and step % args.gradient_accumulation_steps == 0:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if completed_steps % checkpointing_steps == 0 and step % args.gradient_accumulation_steps == 0:
if completed_steps % checkpointing_steps == 0 and accelerator.sync_gradients:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SunMarc your suggestion looks good to me, but I just noticed after committing your change that it doesn't change all scripts. Is there a quick way to fix this?

@eminorhan @SunMarc
Update examples/pytorch/image-classification/run_image_classification…
1c57a74 
…_no_trainer.py

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
SunMarc
SunMarc approved these changes Aug 27, 2024
View reviewed changes
Copy link
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.

Here you go ! I added the remaining suggestion

eminorhan and others added 14 commits August 27, 2024 09:27
@eminorhan @SunMarc
Update examples/pytorch/translation/run_translation_no_trainer.py
10f3aed 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/token-classification/run_ner_no_trainer.py
c533d41 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/text-classification/run_glue_no_trainer.py
cd7b366 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/summarization/run_summarization_no_trainer.py
01e941b 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/semantic-segmentation/run_semantic_segmentati…
8eaa23c 
…on_no_trainer.py

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/language-modeling/run_mlm_no_trainer.py
ac63abe 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/language-modeling/run_fim_no_trainer.py
e25fcd4 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/language-modeling/run_clm_no_trainer.py
7a5b2df 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/image-pretraining/run_mim_no_trainer.py
7f87ee6 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/instance-segmentation/run_instance_segmentati…
51dbd37 
…on_no_trainer.py

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/multiple-choice/run_swag_no_trainer.py
72be88c 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/question-answering/run_qa_no_trainer.py
823ff12 
Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/object-detection/run_object_detection_no_trai…
3bffa0f 
…ner.py

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@eminorhan @SunMarc
Update examples/pytorch/question-answering/run_qa_beam_search_no_trai…
f91c518 
…ner.py

Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com>
@SunMarc SunMarc merged commit d47a9e8 into huggingface:main Aug 27, 2024
7 checks passed
@HuggingFaceDocBuilderDev
Copy link

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.

@eminorhan eminorhan deleted the remove-redundant-checkpointing branch August 27, 2024 17:54
BernardZach pushed a commit to BernardZach/transformers that referenced this pull request Dec 5, 2024
@eminorhan @SunMarc @BernardZach
fix redundant checkpointing in example training scripts (huggingface#…
9df6aa9 
…33131)

* fix redundant checkpointing in example scripts

* Update examples/pytorch/image-classification/run_image_classification_no_trainer.py

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

* Update examples/pytorch/translation/run_translation_no_trainer.py

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

* Update examples/pytorch/token-classification/run_ner_no_trainer.py

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

* Update examples/pytorch/text-classification/run_glue_no_trainer.py

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

* Update examples/pytorch/summarization/run_summarization_no_trainer.py

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

* Update examples/pytorch/semantic-segmentation/run_semantic_segmentation_no_trainer.py

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

* Update examples/pytorch/language-modeling/run_mlm_no_trainer.py

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

* Update examples/pytorch/language-modeling/run_fim_no_trainer.py

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

* Update examples/pytorch/language-modeling/run_clm_no_trainer.py

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

* Update examples/pytorch/image-pretraining/run_mim_no_trainer.py

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

* Update examples/pytorch/instance-segmentation/run_instance_segmentation_no_trainer.py

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

* Update examples/pytorch/multiple-choice/run_swag_no_trainer.py

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

* Update examples/pytorch/question-answering/run_qa_no_trainer.py

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

* Update examples/pytorch/object-detection/run_object_detection_no_trainer.py

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

* Update examples/pytorch/question-answering/run_qa_beam_search_no_trainer.py

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

---------

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
Reviewers

@LysandreJik LysandreJik LysandreJik approved these changes

@SunMarc SunMarc SunMarc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

redundant checkpointing in example scripts
4 participants
@eminorhan @HuggingFaceDocBuilderDev @LysandreJik @SunMarc