Skip to content

Update avg/eta/eta_td only once per second #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 1, 2019

Conversation

pquentin
Copy link
Contributor

When the progress bar is updated frequently (every few milliseconds), the eta can change so quickly that it's impossible to read. This is what happens with pip, for example.

This change means we're calling monotonic/time often, but those calls take less than 100 nsec on Linux, Windows and macOS, which is equivalent to one attribute lookup or two.

@Oxicode
Copy link

Oxicode commented Jan 31, 2019

+1 PR

@verigak
Copy link
Owner

verigak commented Feb 1, 2019

@pquentin You moved the calculation of self.avg outside the if n > 0 block. Can you elaborate why? Also now there is a delay of 1 sec to get the first avg/eta, would it make sense to make an exception for the first second?

@praiskup
Copy link

praiskup commented Feb 1, 2019

It is IMO somewhat related to this commit python-progress@19fd134

@pquentin
Copy link
Contributor Author

pquentin commented Feb 1, 2019

@praiskup it is related, but I tried to send a more minimal fix

@verigak Nice catches! I now update the average while sma_window is being filled, then after every second. I think it's a nice compromise. What do you think?

@verigak
Copy link
Owner

verigak commented Feb 1, 2019

Thanks, that looks good! Can you squash the last 3 commits into one?

@praiskup
Copy link

praiskup commented Feb 1, 2019

@pquentin ok, but this does't fix the broken algorithm/miscalculation, right? Check carefully whether the progress bar reports a "correct" speed (or whether it just makes it a bit readable).

When the progress bar is updated frequently (every few milliseconds),
the eta can change so quickly that it's impossible to read.

This commit updates the average while sma_window is being filled, then
after every second.

This means we're calling monotonic/time often, but those calls take less
than 100 nsec per loop on Linux, Windows and macOS [0], which is
equivalent to one attribute lookup or two.

[0]: python-trio/trio#33
@pquentin
Copy link
Contributor Author

pquentin commented Feb 1, 2019

@praiskup I did not read #24 in detail, but as far as I can tell the simple moving average algorithm is correct, the issue is that progress only uses the latest ten updates, and with pip each update is about 10kB. The output could be more precise with an exponential moving average or by using more samples, but that's a different problem in my opinion.

@verigak Thanks, done!

@verigak verigak merged commit 61627a8 into verigak:master Feb 1, 2019
@verigak
Copy link
Owner

verigak commented Feb 1, 2019

Thanks!

@pquentin pquentin deleted the throttle-eta branch February 2, 2019 05:51
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.

4 participants