Missing the number of replicas in performance estimate #578
Conversation
| @@ -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 * \ | |||
There was a problem hiding this comment.
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)?
| @@ -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): | |||
There was a problem hiding this comment.
Add a docstring (I know it is a private method but I it would be nice) describing what this method does.
| 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) |
There was a problem hiding this comment.
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.
|
@mikemhenry Thanks for the comments, I've checked the numbers with the |
Description
The number of replicas was a factor missing in the estimation of the raw performance for the real time analysis output.
Todos
Status