-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Rename sanity_checks to confidence_checks #5201
Conversation
allennlp/training/trainer.py
Outdated
@@ -294,7 +302,8 @@ def __init__( | |||
num_gradient_accumulation_steps: int = 1, | |||
use_amp: bool = False, | |||
enable_default_callbacks: bool = True, | |||
run_sanity_checks: bool = True, | |||
run_sanity_checks: Optional[bool] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's better to just check for this in **kwargs
? Either way works, I just thought we had gone with the "kwargs" approach elsewhere. But I don't feel strongly either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I like the **kwargs
approach. Updated, along with other suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Thanks :)
* renaming sanity_checks to confidence_checks * update changelog * docs fix * clean up
Related: allenai/allennlp-models#262