Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Missing the number of replicas in performance estimate #578

Merged
merged 5 commits into from May 27, 2022
Merged

Missing the number of replicas in performance estimate #578

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e9a692e
Missed the number of replicas in performance estimate. Whoops!
ijpulidos Apr 19, 2022
2412624
Merge branch 'main' into fix-report-ns-per-day
ijpulidos Apr 19, 2022
42a6f45
Including all moves in performance estimation.
ijpulidos May 25, 2022
096f390
Merge branch 'main' into fix-report-ns-per-day
ijpulidos May 25, 2022
45528b3
Fixing time estimates. Using units. Release notes info.
ijpulidos May 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion openmmtools/multistate/multistatesampler.py
View file
Open in desktop
Expand Up @@ -1759,7 +1759,8 @@ def _update_timing(self, iteration_time, partial_total_time, run_initial_iterati
# TODO: use units for timing information to easily convert between seconds and days
# there are some mcmc_moves that have no timestep attribute, catch exception
try:
current_simulated_time = self.mcmc_moves[0].timestep * self._iteration * self.mcmc_moves[0].n_steps
current_simulated_time = self.mcmc_moves[0].timestep * self._iteration * self.mcmc_moves[0].n_steps * \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we actually be summing over all mcmc_moves, in case there are multiple MCMCMove objects with dynamics involved, or appearing in different order?

And something still seems off here---do we want to use the estimate from average_seconds_per_iteration instead of the partial_total_time (which is the accumulated time since run() was started)?

self.n_replicas
except AttributeError:
current_simulated_time = 0.0 * unit.nanosecond # Hardcoding to 0 ns
performance = current_simulated_time.in_units_of(unit.nanosecond) / (partial_total_time / 86400)
Expand Down