-
Notifications
You must be signed in to change notification settings - Fork 85
Remove restriction on online_analysis_interval and checkpoint_interval #779
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
Remove restriction on online_analysis_interval and checkpoint_interval #779
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
🚀 New features to boost your workflow:
|
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 is a great one line change, thanks @ianmkenney !
if self.online_analysis_interval: | ||
if self.online_analysis_interval % self._reporter.checkpoint_interval != 0: | ||
raise ValueError(f"Online analysis interval: {self.online_analysis_interval}, must be a " | ||
f"multiple of the checkpoint interval: {self._reporter.checkpoint_interval}") | ||
logger.warning("An online_analysis_interval that is not a multiple of the checkpoint_interval can lead to redundant information in the real time yaml file.") |
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.
"after recovering from checkpoints?" or something like that?
Specify that redundant information is added to the real time analysis file only if recovering from a checkpoint older than that information.
@ijpulidos ready for review! |
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, this looks great!
Description
This PR removes the requirement that the
online_analysis_interval
is a multiple of thecheckpoint_interval
.closes #770
Todos
Status
Changelog message