Skip to content

Conversation

calpt
Copy link
Contributor

@calpt calpt commented May 2, 2022

What does this PR do?

This PR does a small refactoring in the Trainer class, specifically it moves the logic for the following two steps out of the training loop into separate helper methods:

  • loading a pre-existing checkpoint into the Trainer before the training starts is moved into the _load_from_checkpoint() method.
  • loading the best evaluated model checkpoint after training has completed is moved into the _load_best_model() method.

The PR does not change any existing logic in any way.

Motivation

In our library, we implement a custom Trainer class that subclasses your great built-in Trainer class. However, as we don't save full model checkpoints during training, the mentioned steps for checkpoint loading are not applicable to our use case. Moving this logic to separate methods would be super helpful to us (and potentially others), since we could easily override these helper methods without modifying the training loop itself.

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.

@sgugger

(cc @hSterz)

@calpt calpt marked this pull request as ready for review May 2, 2022 13:18
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 2, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Collaborator

@sgugger sgugger 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 the PR! This will also make the code of the train method easier to read.

Note that you might have to keep up to date with some of the changes we do in those methods (I actually have a bug to fix to resume from sharded chekpoints that will make some changes in your two new private methods) when you subclass in your Trainer.

@sgugger sgugger merged commit daecae1 into huggingface:main May 2, 2022
stevhliu pushed a commit to stevhliu/transformers that referenced this pull request May 3, 2022
elusenji pushed a commit to elusenji/transformers that referenced this pull request Jun 12, 2022
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.

3 participants