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 4 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
19 changes: 13 additions & 6 deletions openmmtools/multistate/multistatesampler.py
View file
Open in desktop
Expand Up @@ -1758,12 +1758,10 @@ def _update_timing(self, iteration_time, partial_total_time, run_initial_iterati
# Estimate performance
# 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
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)
self._timing_data["ns_per_day"] = performance.value_in_unit(unit.nanosecond)
moves_iterator = self._flatten_moves_iterator()
current_simulated_nanoseconds = sum([move.timestep.value_in_unit(unit.nanosecond) * move.n_steps for
move in moves_iterator if hasattr(move, "timestep") and hasattr(move, "n_steps")])
self._timing_data["ns_per_day"] = current_simulated_nanoseconds / (partial_total_time / 86400)
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to add something like 86400 (seconds in a day) or something -- not everyone will know what that number means. Even better would be to use unit to do the conversion since we are using the line above it.


@staticmethod
def _display_cuda_devices():
Expand All @@ -1774,6 +1772,15 @@ def _display_cuda_devices():
cuda_devices_list = [entry.split(',') for entry in cuda_query_output.split('\n')]
logger.debug(f"CUDA devices available: {*cuda_devices_list,}")

def _flatten_moves_iterator(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a docstring (I know it is a private method but I it would be nice) describing what this method does.

def flatten(iterator):
try:
yield from [inner_move for move in iterator for inner_move in flatten(move)]
except TypeError:
logger.warning(f"Could not flatten {type(iterator).__name__} object!")
yield iterator
return flatten(self.mcmc_moves)

# -------------------------------------------------------------------------
# Internal-usage: Test globals
# -------------------------------------------------------------------------
Expand Down