-
Notifications
You must be signed in to change notification settings - Fork 2
[WIP]Enh/checkpointing #8
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
base: master
Are you sure you want to change the base?
Conversation
mschmidt87
left a comment
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.
Looks good. Just some minor comments.
|
|
||
| if load_existing_checkpoint: | ||
| state = load_checkpoint() | ||
| if state: |
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.
If state is None here, i.e. if there were no checkpoints that can be loaded, I suggest to raise an error. Otherwise checkpoint loading fails silently.
| from . import lib | ||
|
|
||
|
|
||
| def load_checkpoint(): |
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.
What about enabling the user to specify a specific checkpoint, not only the most recent one? You could implement this by allowing load_existing_checkpoint to be specified as True or -1 (for the most recent one) or as an integer specifying the generation.
| pickle.dump(state, f) | ||
|
|
||
|
|
||
| def extract_keys_from_dict(d, whitelist, blacklist=[]): |
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.
It appears to me that this function should be located in lib.py.
This PR adds basic checkpointing by dumping all local variables that are changed during optimization to a pickle file and automatically loading the most recent checkpoint if any is found.
During the implementation, I found that it may be much easier to implement checkpointing if the optimization routine is a stateful object, so maybe your idea @mschmidt87 to turn all algorithms into classes should be done first and a new checkpointing mechanism added to that. Let me know what you think about this.
Branches of #4, so should not be merged before that.