pele-python / mcpele

Monte Carlo and parallel tempering routines built on the pele foundation
Other
20 stars 5 forks source link

class progress is confusing #9

Closed js850 closed 10 years ago

js850 commented 10 years ago

the class progress is extremely confusing. I have no real idea what it's doing apart from printing something to do with time and progress. The member variables oom and m should be named something that indicates what they do.

TimePercentage should be renamed something like print_time_percentage, becase that's what it actually does

IntToTime seems way more complicated than just converting an integer to time. are there not standard library functions to do that? Same with PrintTime.

smcantab commented 10 years ago

Boost has already a class progress_display, so we could just use that

http://www.boost.org/doc/libs/1_52_0/libs/timer/doc/original_timer.html#Class%20progress_display

if we have to clean it up we might as well play with carriage return and have everything printed on the same line, maybe have a "load bar" sort of output. This seems like a link that might help

https://stackoverflow.com/questions/14539867/how-to-display-a-progress-indicator-in-pure-c-c-cout-printf

On Fri, Jul 4, 2014 at 11:13 AM, Jacob Stevenson notifications@github.com wrote:

the class progress is extremely confusing. I have no real idea what it's doing apart from printing something to do with time and progress. The member variables oom and m should be named something that indicates what they do.

TimePercentage should be renamed something like print_time_percentage, becase that's what it actually does

IntToTime seems way more complicated than just converting an integer to time. are there not standard library functions to do that? Same with PrintTime.

— Reply to this email directly or view it on GitHub https://github.com/pele-python/mcpele/issues/9.

kjs73 commented 10 years ago

Yes progress class can be deleted or improved. I only included it so that one does not have to wait for the simulation to finish to know how long it will take. If there is something in boost that does it, we should use that instead.

On Friday, 4 July 2014, smcantab notifications@github.com wrote:

Boost has already a class progress_display, so we could just use that

http://www.boost.org/doc/libs/1_52_0/libs/timer/doc/original_timer.html#Class%20progress_display

if we have to clean it up we might as well play with carriage return and have everything printed on the same line, maybe have a "load bar" sort of output. This seems like a link that might help

https://stackoverflow.com/questions/14539867/how-to-display-a-progress-indicator-in-pure-c-c-cout-printf

On Fri, Jul 4, 2014 at 11:13 AM, Jacob Stevenson <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

the class progress is extremely confusing. I have no real idea what it's doing apart from printing something to do with time and progress. The member variables oom and m should be named something that indicates what they do.

TimePercentage should be renamed something like print_time_percentage, becase that's what it actually does

IntToTime seems way more complicated than just converting an integer to time. are there not standard library functions to do that? Same with PrintTime.

— Reply to this email directly or view it on GitHub https://github.com/pele-python/mcpele/issues/9.

— Reply to this email directly or view it on GitHub https://github.com/pele-python/mcpele/issues/9#issuecomment-48028576.

kjs73 commented 10 years ago

At the time, I did not find a library function that prints time properly, like with days, hours, etc. and only putting a plural s if there are more than one hours etc.

On Friday, 4 July 2014, Julian Schrenk kjs73@cam.ac.uk wrote:

Yes progress class can be deleted or improved. I only included it so that one does not have to wait for the simulation to finish to know how long it will take. If there is something in boost that does it, we should use that instead.

On Friday, 4 July 2014, smcantab <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Boost has already a class progress_display, so we could just use that

http://www.boost.org/doc/libs/1_52_0/libs/timer/doc/original_timer.html#Class%20progress_display

if we have to clean it up we might as well play with carriage return and have everything printed on the same line, maybe have a "load bar" sort of output. This seems like a link that might help

https://stackoverflow.com/questions/14539867/how-to-display-a-progress-indicator-in-pure-c-c-cout-printf

On Fri, Jul 4, 2014 at 11:13 AM, Jacob Stevenson < notifications@github.com> wrote:

the class progress is extremely confusing. I have no real idea what it's doing apart from printing something to do with time and progress. The member variables oom and m should be named something that indicates what they do.

TimePercentage should be renamed something like print_time_percentage, becase that's what it actually does

IntToTime seems way more complicated than just converting an integer to time. are there not standard library functions to do that? Same with PrintTime.

— Reply to this email directly or view it on GitHub https://github.com/pele-python/mcpele/issues/9.

— Reply to this email directly or view it on GitHub https://github.com/pele-python/mcpele/issues/9#issuecomment-48028576.

kjs73 commented 10 years ago

As far as I can tell, the boost progress bar does not implement time prediction. It seems to me that it is not worth adding the boost dependence for just the progress bar.

Thanks for your feedback, I will try to make progress clearer accordingly.