-
Notifications
You must be signed in to change notification settings - Fork 42
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
ModelCheckpoint must be defined in the config dict, not during the parsing. #454
Conversation
…rsing Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida given @blumenstiel 's feedback, do you want to close this PR or merge it as an interim fix (which we can do, if we create an issue for the proper refactoring which we can push to v1.1 for example) |
@romeokienzler @Joao-L-S-Almeida With this fix, the user cannot overwirte the default settings. So I don't think you should merge it with the current code. At least, check if the user defines a checkpoint and only add if it is missing. |
Right. We can do it. |
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@blumenstiel @romeokienzler I added it as part of the callbacks defined in the yaml file. When no callback is provided, but |
The file |
Signed-off-by: Joao Lucas de Sousa Almeida <[email protected]>
Thanks @Joao-L-S-Almeida, looks good. @romeokienzler Can you have another look and test it? |
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
@blumenstiel seems your approval is needed as well |
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
Benedikt indicated in the comments that he is fine with the changes
It should avoid the issue: