Fixes issue #580 #722
Open
Fixes issue #580 #722
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.