-
Notifications
You must be signed in to change notification settings - Fork 25
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
Feature / Request History options #1141
Conversation
…y shouldn't have double quotes. Plus this makes the tabs resemble the autodoc_pydantic output more. Did this while initially adding the new History options
… save_H_on_completion is set
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.
Thanks for this @jlnav!
I have given it a try and it works very well. There's only an additional comment I have, which is that the file saved on completion has a different name than those saved every k steps. Ideally this wouldn't be the case, because then we always have two history files after an optimas run (e.g., exploration_history_after_sim_20.npy
and exploration_history_after_completion.npy
).
One possible solution that I see would be a final call to
if self.libE_specs.get("save_every_k_sims"):
self._save_every_k_sims()
if self.libE_specs.get("save_every_k_gens"):
self._save_every_k_gens()
if "save_H_on_completion"
is enabled. This would probably require a force=True
argument in _save_every_k
to make sure that the final history is saved.
Sounds good, thanks! I can make that change right away. |
…filenames should be identical to normal save_every_k files
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.
Works great now :D
…ve_every_k_sims is not set
If save_H_on_completion defaults to True, what stops you from getting two .npy files at the end, when user is dumping their file already? We need to agree on defaults. One consideration is multiple calls to libE, they might only want to dump at the end. |
Addresses #1103
New options plus their defaults: