[Data] Make sure progress bars always finish at 100% #36679
Conversation
Signed-off-by: Hao Chen <chenh1024@gmail.com>
raulchen
requested review from
ericl,
scv119,
c21,
amogkam,
scottjlee and
bveeramani
as code owners
raulchen
changed the title
| @@ -105,11 +107,17 @@ def set_description(self, name: str) -> None: | |||
| self._bar.set_description(self._desc) | |||
|
|
|||
| def update(self, i: int) -> None: | |||
| # Make sure the progress bar doesn't go over 100%. | |||
| i = min(i, self._total - self._position) | |||
There was a problem hiding this comment.
I think we do want to show it going over 100%, otherwise it will look "stuck" at 100%, right? Maybe in that case we should change the total to be higher?
There was a problem hiding this comment.
ah right, I think we could change self._total to reflect the progress.
There was a problem hiding this comment.
I think you might have to also update self._bar's total somehow. I'm not sure about the tqdm API here, might need to try it out.
There was a problem hiding this comment.
If not possible, going over 100% is OK in tqdm as well, it just goes from a progress bar to a plain meter when you exceed 100.
There was a problem hiding this comment.
I didn't find an API, but this seems to work well: bar.total = new_total; bar.refresh().
However, even if we update the total, the bar will still stuck at 100%, right? The only benefit is the progress value will change.
Do you guys prefer this?
There was a problem hiding this comment.
By the way, do you know what is the simplest way to reproduce the over 100% case?
There was a problem hiding this comment.
I end up updating the total. I think this is useful, as we can see the actual number of blocks from here.
Signed-off-by: Hao Chen <chenh1024@gmail.com>
|
Merging in master where a couple of the test failures have been resolved :) |
This is a bug introduced by #36679. I thought `position` was the bar position (initial progress). But it is actually the line position to print the bar. Rename the variable to "progress", and default initial value to 0. Signed-off-by: Hao Chen <chenh1024@gmail.com>
Today the progress bar's total progress is defined by the number of estimated blocks. If the estimation is not correct, the progress bar may go over 100% or finishes half way. This PR make sure the progress bar always finishes at 100%. Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
This is a bug introduced by ray-project#36679. I thought `position` was the bar position (initial progress). But it is actually the line position to print the bar. Rename the variable to "progress", and default initial value to 0. Signed-off-by: Hao Chen <chenh1024@gmail.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Why are these changes needed?
Today the progress bar's total progress is defined by the number of estimated blocks. If the estimation is not correct, the progress bar may go over 100% or finishes half way. This PR make sure the progress bar always finishes at 100%.
Related issue number
#36490
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.