8000 Remove restriction on online_analysis_interval and checkpoint_interval by ianmkenney · Pull Request #779 · choderalab/openmmtools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 4 commits into from
May 21, 2025

Conversation

ianmkenney
Copy link
Contributor
@ianmkenney ianmkenney commented Apr 30, 2025

Description

This PR removes the requirement that the online_analysis_interval is a multiple of the checkpoint_interval.

closes #770

Todos

  • Implement feature / fix bug
  • Add tests
  • Update documentation as needed
  • Update changelog to summarize changes in behavior, enhancements, and bugfixes implemented in this PR

Status

  • Ready to go

Changelog message

Copy link
codecov bot commented Apr 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.16%. Comparing base (6a9d5d3) to head (33535b2).
Report is 1 commits behind head on main.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor
@IAlibay IAlibay left a 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.")
Copy link
Contributor

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?

ianmkenney added 3 commits May 1, 2025 09:37
Specify that redundant information is added to the real time analysis
file only if recovering from a checkpoint older than that information.
@ianmkenney ianmkenney marked this pull request as ready for review May 1, 2025 17:27
@ianmkenney ianmkenney requested a review from IAlibay May 1, 2025 17:28
@ianmkenney
Copy link
Contributor Author

@ijpulidos ready for review!

Copy link
Contributor
@ijpulidos ijpulidos left a 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!

@ijpulidos ijpulidos merged commit 4f76b7b into choderalab:main May 21, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decouple checkpoint_interval and online_analysis_interval
3 participants
0