Fixes issue #580#722
Conversation
Simplify ProgressState.duration() function to show the duration regardless of if there is no progress or if the progressbar is finished. Rename Estimator.prev_steps to steps_done for increased readability
…as been made in a way where every second could be the second progress gets made so we assume each step needed will take that much time. This shows in an ETA that grows while no progress has been made after starting a progress bar and settles when the first step is made. The "sanity check" in Estimator.record() function needed to be removed for this since it prohibited any updates to Estimator parameters if no progress had been made. The ProgressState.eta() now also includes the time passed since last progress was made in it's ETA calculations and that is what makes the ETA counter count down. Add sec_per_step to Estimator which is an estimate for how many seconds each step takes. Also included sec_per_step in the Estimator.reset() function. Update Estimator.record() function to keep returning the same estimate for how many seconds each step takes even though no progress has been made. This is done so that the ProgressState.eta() function will keep counting down the actual estimated time. An exception is if the time since the last progress is more than the average time per step then we increase the estimate for how many seconds each step takes by 2% everytime Estimator.record() functino is called. This shows an increase in the ETA at a rate that is much more conservative than the previous rate and in the case of no progress being possible (for example if the progress is for a transmission and the router is unplugged) then the user will clearly see the ETA rise with no hope of it going down. In the case that progress does get done (somebody plugged the router back in) then the actual time it took for progress to be made is used to estimate the time for future steps. These changes make the parameters Estimator.smoothed_steps_per_sec and Estimator.double_smoothed_steps_per_sec obsolete and unnecessary as well as some functions potentially, like Estimator.steps_per_second() which uses this double exponential smoothing and seems to be only used in the tests/examples and also in the ProgressBar.per_sec() function which I don't know how much is used so I left everything related to these parameters untouched.
|
@djc How is this? |
|
It's on my list -- not sure when I'll be able to get to it, but pretty sure I will at some point. |
|
Great, yeah no stress from my part. |
|
There were apparently 2 places I missed with the |
|
Apparently the |
|
@djc can we merge this? |
|
I'm sorry, but this is not a high priority for me. It is still on my list to review, but it might take time for me to get to it. |
|
All righty, no stress, Was just setting up a program on another computer and was reminded that I'm still using my fork of the repo instead of the official one. |
2 participants
Here is a new and improved pull request which contains 2 commits.
The first one is from when I was mostly gaining understanding of the code and the second one contains the changes I implemented.
Here is the old pull request:
#721
This should fix issue #580
Detailed explanation
Change ProgressState.eta() function to show ETA before any progress has been made in a way where every second could be the second progress gets made so we assume each step needed will take that much time. This shows in an ETA that grows while no progress has been made after starting a progress bar and settles when the first step is made.
The "sanity check" in Estimator.record() function needed to be removed for this since it prohibited any updates to Estimator parameters if no progress had been made.
The ProgressState.eta() now also includes the time passed since last progress was made in it's ETA calculations and that is what makes the ETA counter count down.
Add sec_per_step to Estimator which is an estimate for how many seconds each step takes.
Also included sec_per_step in the Estimator.reset() function.
Update Estimator.record() function to keep returning the same estimate for how many seconds each step takes even though no progress has been made. This is done so that the ProgressState.eta() function will keep counting down the actual estimated time.
An exception is if the time since the last progress is more than the average time per step then we increase the estimate for how many seconds each step takes by 2% everytime Estimator.record() functino is called. This shows an increase in the ETA at a rate that is much more conservative than the previous rate and in the case of no progress being possible (for example if the progress is for a transmission and the router is unplugged) then the user will clearly see the ETA rise with no hope of it going down. In the case that progress does get done (somebody plugged the router back in) then the actual time it took for progress to be made is used to estimate the time for future steps.
These changes make the parameters Estimator.smoothed_steps_per_sec and Estimator.double_smoothed_steps_per_sec obsolete and unnecessary as well as some functions potentially, like Estimator.steps_per_second() which uses this double exponential smoothing and seems to be only used in the tests/examples and also in the ProgressBar.per_sec() function which I don't know how much is used so I left everything related to these parameters untouched.