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.

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

Load best model + evaluate best model on devset #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

OrianeN
Copy link

@OrianeN OrianeN commented Jan 22, 2025

I've noticed that the model loaded at the end of training differs whether the EarlyStoppingException was raised or not:

  • if EarlyStoppingException: load best model
  • otherwise, keep on with the last model

I'd like to suggest always loading the best model at the end of the training phase, as well as performing the final evaluation successively on the devset and on the testset (with the best model).

Logs of 2 runs to show you the changes in practice:

Note: I've never tried training without a dev set, so I'm not sure about the comment at the end of the Trainer.train_epochs() function:
https://github.com/OrianeN/PaPie/blob/d23f4a5e95638e74dad640cf7f1d44b896c2fb0e/pie/trainer.py#L429-L430

The best model was only loaded in the case of early stopping,
now it is loaded at the end of all trainings.

Also, the final evaluation is performed with the best model
1) on the dev set
2) on the test set
@OrianeN OrianeN marked this pull request as draft January 22, 2025 12:17
@OrianeN
Copy link
Author

OrianeN commented Jan 22, 2025

As discussed, I made the training script save both the best and the last model, as well as print the index of the best epoch (starting from 1).

Here are the updated example logs: papie_run_loadbest_saveboth.log

@OrianeN OrianeN marked this pull request as ready for review January 22, 2025 12:38
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