Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Configure validation frequency #5534

Merged
merged 13 commits into from
Jan 4, 2022

Conversation

JohnGiorgi
Copy link
Contributor

@JohnGiorgi JohnGiorgi commented Jan 3, 2022

Fixes #5214 .

Changes proposed in this pull request:

  • Add a boolean flag to GradientDescentTrainer (_should_validate_this_epoch) that controls whether validation will occur at the end of an epoch.
  • Add a callback, ShouldValidateCallback that allows a user to configure validation frequency.
  • The callback address both points in Easy way to run evaluation every n-th epoch? #5214, by allowing you to delay validation until a certain number of epochs have elapsed (with validation_start) and the validation frequency (with validation_interval).

Before submitting

  • I've read and followed all steps in the Making a pull request
    section of the CONTRIBUTING docs.
  • I've updated or added any relevant docstrings following the syntax described in the
    Writing docstrings section of the CONTRIBUTING docs.
  • If this PR fixes a bug, I've added a test that will fail without my fix.
  • If this PR adds a new feature, I've added tests that sufficiently cover my new functionality.

After submitting

  • All GitHub Actions jobs for my pull request have passed.
  • codecov/patch reports high test coverage (at least 90%).
    You can find this under the "Actions" tab of the pull request once the other checks have finished.

@JohnGiorgi
Copy link
Contributor Author

@epwalsh Mind taking a look when you get a chance? This is a first attempt to address both points made in #5214!

Copy link
Member

@epwalsh epwalsh left a comment

Choose a reason for hiding this comment

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

Thanks @JohnGiorgi, this seems like a good approach. I have a couple comments 👇

class ShouldValidateCallback(TrainerCallback):
"""
A callback that you can pass to the `GradientDescentTrainer` to change the frequency of
validation during training. If `start_validation` is not `None`, validation will not occur until
Copy link
Member

Choose a reason for hiding this comment

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

I think validation_start sounds a little better than start_validation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed! Yeah, I was struggling with the naming, if you have any other name change suggestions let me know!

tests/training/trainer_test.py Outdated Show resolved Hide resolved
Co-authored-by: Pete <epwalsh10@gmail.com>
Copy link
Member

@epwalsh epwalsh left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @JohnGiorgi!

@JohnGiorgi
Copy link
Contributor Author

LGTM! Thanks @JohnGiorgi!

Cool! Thanks @epwalsh

@epwalsh epwalsh enabled auto-merge (squash) January 4, 2022 19:16
@epwalsh epwalsh merged commit 515fe9b into allenai:main Jan 4, 2022
@JohnGiorgi JohnGiorgi deleted the configure-validation-frequency branch January 4, 2022 20:09
JohnGiorgi added a commit to JohnGiorgi/allennlp that referenced this pull request Jan 4, 2022
dirkgr added a commit that referenced this pull request Jan 12, 2022
Co-authored-by: Dirk Groeneveld <dirkg@allenai.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants