Skip to content

Improvements on progress output - #676

Merged
jhprinz merged 6 commits into
openpathsampling:masterfrom
dwhswenson:update_progress_output
Apr 6, 2017
Merged

jhprinz merged 6 commits into
openpathsampling:masterfrom
dwhswenson:update_progress_output

Conversation

@dwhswenson

@dwhswenson dwhswenson commented Mar 29, 2017 •

Copy link
Copy Markdown
Member

This PR fixes a few minor annoyances with the progress output during a path sampling simulation. In particular, it includes the following:

  • Switch the speed reporting from "steps per second" to "seconds per step": TPS simulations of large systems have less than 0.01 steps per second, so the old version was uninformative; even our fast systems are around 0.1-0.2 seconds per step
  • Convert times (elapsed and remaining) from seconds to days, hours, minutes, seconds (easier to interpret once you get to tens of thousands of seconds remaining)
  • Move the whole progress string generation into paths.tools, so that it can be re-used by other objects. Just needs the time elapsed, the steps completed, and the total number of steps
  • Unit tests for new code

New output looks something like this:

Working on Monte Carlo cycle number 215
Running for 1 minute 44 seconds -  0.91 seconds per step
Estimated time remaining: 5 hours 2.52 minutes

The time elapsed is always the full string (1 day 2 hours 3 minutes 4 seconds). The time remaining always contains the first two (occupied) "levels" (day, hour, minute, second) with the remainder to 2 decimal places in the last. So (1 day 2.05 hours, or as above, 5 hours 2.52 minutes). I thought this would be the most readable (and it acknowledges that the time elapsed is exact, whereas the time remaining is a rough estimate).

Generalizes and improves (for readability) the output.

Also includes some additional logging in ensemble.py (for the
trajectory-building stuff)
@dwhswenson

Copy link
Copy Markdown
Member Author

This also added some additional logging to Ensemble.extend_sample_from_trajectories that I needed for some debugging I was doing.

@dwhswenson dwhswenson changed the title [WIP] Improvements on progress output Improvements on progress output Apr 5, 2017
@dwhswenson dwhswenson changed the title Improvements on progress output [WIP] Improvements on progress output Apr 5, 2017
@dwhswenson dwhswenson changed the title [WIP] Improvements on progress output Improvements on progress output Apr 5, 2017
@dwhswenson

Copy link
Copy Markdown
Member Author

Now that #681 is in, this passes! Ready for review.

The pretty_print_seconds isn't as pretty of code as it might be (more elegant approaches may exist), but it does some smart rounding (it knows to round "2 days 1 hour 59 minutes 50 seconds" to "2 days 2 hours" -- which includes changing the singular "hour" to plural "hours.") So I'm happy enough with it for now.

@jhprinz

jhprinz commented Apr 6, 2017

Copy link
Copy Markdown
Contributor

Lookgs good to me. The formatting seems to be a lot of code, but I don't know a better solution. Will merge now.

@jhprinz
jhprinz merged commit 2107b76 into openpathsampling:master Apr 6, 2017
@dwhswenson

Copy link
Copy Markdown
Member Author

The formatting seems to be a lot of code

I'm pretty sure there is a better solution, but it would involve a complete rewrite. The problem was that my first version forgot about rounding up, so I kinda had to hack that in. Something to rewrite someday; lots of other things much higher on the priority list!

@dwhswenson
dwhswenson deleted the update_progress_output branch April 6, 2017 17:15
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.

2 participants