Skip to content
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

tqdm progress bar improvements #2114

Merged
merged 10 commits into from
Sep 12, 2023

Conversation

nikita-savelyevv
Copy link
Collaborator

@nikita-savelyevv nikita-savelyevv commented Sep 7, 2023

Changes

  1. Fixed an issue with wrong tqdm bar length in the case when calibration dataset length is less than subset_size.
    Reproducer: nikita-savelyevv@f0951c1
    Before:
    Statistics collection: 34%|██████ | 101/300 [00:03<00:06, 28.66it/s]
    After:
    When dataset has __len__:
    Statistics collection: 100%|██████████████████| 101/101 [00:03<00:00, 28.20it/s]
    When dataset doesn't have __len__:
    Statistics collection: 34%|██████ | 101/300 [00:03<00:06, 29.45it/s]

  2. Improved progress bar GUI when ran from notebooks.
    Before:

Screenshot 2023-09-06 091857

or (in some browsers progress bar takes up multiple lines):

image
After:
Screenshot 2023-09-06 105453

In console the progress bar is the same.

Reason for changes

User experience improvement.

Related tickets

112627

Tests

@nikita-savelyevv nikita-savelyevv requested a review from a team as a code owner September 7, 2023 12:11
@github-actions github-actions bot added NNCF Common Pull request that updates NNCF Common NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF PTQ Pull requests that updates NNCF PTQ labels Sep 7, 2023
@vshampor vshampor added the API Public API-impacting changes label Sep 7, 2023
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #2114 (ebaa19c) into develop (f6d958f) will increase coverage by 35.92%.
Report is 7 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #2114       +/-   ##
============================================
+ Coverage         0   35.92%   +35.92%     
============================================
  Files            0      476      +476     
  Lines            0    42482    +42482     
============================================
+ Hits             0    15263    +15263     
- Misses           0    27219    +27219     
Files Changed Coverage Δ
nncf/common/tensor_statistics/aggregator.py 98.11% <100.00%> (ø)
nncf/data/dataset.py 79.54% <100.00%> (ø)
...antization/algorithms/bias_correction/algorithm.py 93.88% <100.00%> (ø)
...tization/algorithms/channel_alignment/algorithm.py 25.41% <100.00%> (ø)
...ation/algorithms/fast_bias_correction/algorithm.py 90.83% <100.00%> (ø)
.../quantization/algorithms/smooth_quant/algorithm.py 27.84% <100.00%> (ø)

... and 470 files with indirect coverage changes

nncf/data/dataset.py Outdated Show resolved Hide resolved
@nikita-savelyevv
Copy link
Collaborator Author

run pylint pre-commit tests
run tensorflow pre-commit tests
run openvino pre-commit tests
run onnx pre-commit tests

@AlexanderDokuchaev
Copy link
Collaborator

@nikita-savelyevv when i try to fix logs in ci like

Statistics collection:   0%|          | 0/300 [00:00<?, ?it/s]
Statistics collection:   0%|          | 1/300 [00:00<01:13,  4.09it/s]
Statistics collection:   1%|          | 2/300 [00:00<00:50,  5.87it/s]
Statistics collection:   1%|▏         | 4/300 [00:00<00:33,  8.79it/s]
Statistics collection:   2%|▏         | 6/300 [00:00<00:26, 11.30it/s]
Statistics collection:   3%|▎         | 8/300 [00:00<00:24, 11.88it/s]
Statistics collection:   3%|▎         | 10/300 [00:00<00:23, 12.10it/s]
Statistics collection:   4%|▍         | 12/300 [00:01<00:23, 12.26it/s]
Statistics collection:   5%|▍         | 14/300 [00:01<00:23, 12.37it/s]
Statistics collection:   5%|▌         | 16/300 [00:01<00:22, 12.41it/s]
Statistics collection:   6%|▌         | 18/300 [00:01<00:22, 12.41it/s]
Statistics collection:   7%|▋         | 20/300 [00:01<00:20, 13.37it/s]
Statistics collection:   7%|▋         | 22/300 [00:01<00:19, 14.33it/s]
Statistics collection:   8%|▊         | 24/300 [00:01<00:18, 15.16it/s]
Statistics collection:   9%|▊         | 26/300 [00:02<00:17, 15.82it/s]
Statistics collection:   9%|▉         | 28/300 [00:02<00:16, 16.18it/s]
Statistics collection:  10%|█         | 30/300 [00:02<00:16, 16.51it/s]
Statistics collection:  11%|█         | 32/300 [00:02<00:16, 16.75it/s]
Statistics collection:  11%|█▏        | 34/300 [00:02<00:15, 16.82it/s]
Statistics collection:  12%|█▏        | 36/300 [00:02<00:15, 16.88it/s]
Statistics collection:  13%|█▎        | 38/300 [00:02<00:15, 16.95it/s]
Statistics collection:  13%|█▎        | 40/300 [00:02<00:15, 17.12it/s]
Statistics collection:  14%|█▍        | 42/300 [00:02<00:14, 17.24it/s]
Statistics collection:  15%|█▍        | 44/300 [00:03<00:14, 17.25it/s]
Statistics collection:  15%|█▌        | 46/300 [00:03<00:14, 17.17it/s]
Statistics collection:  16%|█▌        | 48/300 [00:03<00:14, 17.14it/s]

And found that progress bar from pip looks good in one line

12:00:27    Downloading python_dateutil-2.8.2-py2.py3-none-any.whl (247 kB)
12:00:27       ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 247.7/247.7 kB 23.5 MB/s eta 0:00:00

What you think to use https://github.com/Textualize/rich#rich-library instead of tqdm, and in notebooks it's looks better.

May be will be better to use rich instead of tqdm?

@nikita-savelyevv @alexsu52 @andrey-churkin @kshpv what you think about it?

@kshpv
Copy link
Collaborator

kshpv commented Sep 8, 2023

@nikita-savelyevv when i try to fix logs in ci like

Statistics collection:   0%|          | 0/300 [00:00<?, ?it/s]
Statistics collection:   0%|          | 1/300 [00:00<01:13,  4.09it/s]
Statistics collection:   1%|          | 2/300 [00:00<00:50,  5.87it/s]
Statistics collection:   1%|▏         | 4/300 [00:00<00:33,  8.79it/s]
Statistics collection:   2%|▏         | 6/300 [00:00<00:26, 11.30it/s]
Statistics collection:   3%|▎         | 8/300 [00:00<00:24, 11.88it/s]
Statistics collection:   3%|▎         | 10/300 [00:00<00:23, 12.10it/s]
Statistics collection:   4%|▍         | 12/300 [00:01<00:23, 12.26it/s]
Statistics collection:   5%|▍         | 14/300 [00:01<00:23, 12.37it/s]
Statistics collection:   5%|▌         | 16/300 [00:01<00:22, 12.41it/s]
Statistics collection:   6%|▌         | 18/300 [00:01<00:22, 12.41it/s]
Statistics collection:   7%|▋         | 20/300 [00:01<00:20, 13.37it/s]
Statistics collection:   7%|▋         | 22/300 [00:01<00:19, 14.33it/s]
Statistics collection:   8%|▊         | 24/300 [00:01<00:18, 15.16it/s]
Statistics collection:   9%|▊         | 26/300 [00:02<00:17, 15.82it/s]
Statistics collection:   9%|▉         | 28/300 [00:02<00:16, 16.18it/s]
Statistics collection:  10%|█         | 30/300 [00:02<00:16, 16.51it/s]
Statistics collection:  11%|█         | 32/300 [00:02<00:16, 16.75it/s]
Statistics collection:  11%|█▏        | 34/300 [00:02<00:15, 16.82it/s]
Statistics collection:  12%|█▏        | 36/300 [00:02<00:15, 16.88it/s]
Statistics collection:  13%|█▎        | 38/300 [00:02<00:15, 16.95it/s]
Statistics collection:  13%|█▎        | 40/300 [00:02<00:15, 17.12it/s]
Statistics collection:  14%|█▍        | 42/300 [00:02<00:14, 17.24it/s]
Statistics collection:  15%|█▍        | 44/300 [00:03<00:14, 17.25it/s]
Statistics collection:  15%|█▌        | 46/300 [00:03<00:14, 17.17it/s]
Statistics collection:  16%|█▌        | 48/300 [00:03<00:14, 17.14it/s]

And found that progress bar from pip looks good in one line

12:00:27    Downloading python_dateutil-2.8.2-py2.py3-none-any.whl (247 kB)
12:00:27       ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 247.7/247.7 kB 23.5 MB/s eta 0:00:00

What you think to use https://github.com/Textualize/rich#rich-library instead of tqdm, and in notebooks it's looks better.

May be will be better to use rich instead of tqdm?

@nikita-savelyevv @alexsu52 @andrey-churkin @kshpv what you think about it?

Looks nice!

@vshampor
Copy link
Contributor

vshampor commented Sep 8, 2023

@AlexanderDokuchaev perhaps this is not applicable to progress bars exactly, but rich install an own logging handler in order to work which may mess with NNCF logging logic, so before switching to rich, we need to make sure that the rest of the NNCF logging is not impacted, in samples and in package code.

@nikita-savelyevv
Copy link
Collaborator Author

@AlexanderDokuchaev Thanks for the suggestion. I propose to add rich progress bar in a separate PR because it requires some additional customization, for example the default progress bar from rich doesn't have an iteration counter, only percentage and estimated remaining time:
Working... ━━━━━━━━━━╺━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 25% 0:00:39
The iteration counter can be implemented, but requires addition of some custom code.

So let's merge this PR first, I'll then open another for rich.

tests/openvino/datasets_helpers.py Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the NNCF OpenVINO Pull requests that updates NNCF OpenVINO label Sep 11, 2023
@andrey-churkin andrey-churkin merged commit 0a08966 into openvinotoolkit:develop Sep 12, 2023
@nikita-savelyevv nikita-savelyevv deleted the tqdm-auto branch September 25, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Public API-impacting changes NNCF Common Pull request that updates NNCF Common NNCF PTQ Pull requests that updates NNCF PTQ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants