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

Replacing our progress bar with tqdm #398

Closed
BoPeng opened this issue Feb 2, 2017 · 14 comments
Closed

Replacing our progress bar with tqdm #398

BoPeng opened this issue Feb 2, 2017 · 14 comments

Comments

@BoPeng
Copy link
Contributor

BoPeng commented Feb 2, 2017

We have done a lot to use progress bar in a multiprocessing setting, but tqdm seem to also have such a problem. However, it makes sense to use tqdm, which is supposed to have less overhead (at least they take overhead seriously) than our progress bar.

Another side of the story is 'it is not broken, do not fix it'.

@gaow
Copy link
Member

gaow commented Feb 2, 2017

It is appealing if by using tqdm we can make SoS source code shorter :)

@BoPeng
Copy link
Contributor Author

BoPeng commented Feb 2, 2017

Yeah, and it should be easy to replace, especially with the position parameter for multi-processing (or multi-file download).

I will test this module when I get a chance (I am testing simuPOP for its next release), you can do it if you feel like coding now.

@gaow
Copy link
Member

gaow commented Feb 3, 2017

Trying it out -- the problem below is not always reproducible but is rather weird:

Exception in thread Thread-3::1:  67%|█████████████████████████████████████████████▎                      | 2/3 [00:00<00:00,  9.44it/s]
Traceback (most recent call last):
  File "/opt/miniconda3/lib/python3.5/threading.py", line 914, in _bootstrap_inner
    self.run()
  File "/opt/miniconda3/lib/python3.5/site-packages/sos-0.9.3.1-py3.5.egg/sos/utils.py", line 799, in run
    self.term = blessings.Terminal(stream=sys.stderr)
AttributeError: module 'blessings' has no attribute 'Terminal'

"AttributeError: module 'blessings' has no attribute 'Terminal'" is apparently not true. But I do not understand why it complains and why it is not reproducible at all times. I am sure you've tested the new progress bar but you did not see the problem.

@gaow gaow reopened this Feb 3, 2017
@BoPeng
Copy link
Contributor Author

BoPeng commented Feb 3, 2017 via email

@gaow
Copy link
Member

gaow commented Feb 3, 2017

On my end progress bars still are out of place running multiple pipelines. It looks like it does not refresh properly -- in the end I expect one 100% solid bar but I get multiple lines, some empty, some 33%, some 67% and in the end 100%.

@BoPeng
Copy link
Contributor Author

BoPeng commented Feb 3, 2017

Should be fine now.

@BoPeng
Copy link
Contributor Author

BoPeng commented Feb 3, 2017

It is good that tqdm has everything that we need, although I still filed a bug for it.

@gaow
Copy link
Member

gaow commented Feb 7, 2017

There is a slight issue, eg running this:

[test_1]
run:
 touch 1.txt

[test_2]
run:
 touch 2.txt

[test_3]
run:
 touch 3.txt

[default_1]
sos_run('test:1+test:2')

[default_2]
sos_run('test:1+test:3')

with sos run test.sos -v1 the progress bar does not properly flush the console.

@gaow gaow reopened this Feb 7, 2017
@BoPeng
Copy link
Contributor Author

BoPeng commented Feb 8, 2017

This problem is extremely tricky and tqdm has had a lot of discussions on such things. The problem here I guess is that the master progress bar finishes a return but it returns to the next line, not a newline. You can submit a bug report to tqdm with the following code and see what the authors would say

from tqdm import trange
from time import sleep
for i in trange(4, desc='Master'):
    for j in trange(10, desc='nested'):
        sleep(0.1)

In their own example, they added

print('\ndone')

at the end. Note the \n here.

@gaow
Copy link
Member

gaow commented Feb 8, 2017

Yes I think this is their current solution as of 5 days ago:

tqdm/tqdm#230

Is it what we should too adapt? Even with '\n' the outcome is still ugly because bar from the last nested instance will be kept under the master.

@BoPeng
Copy link
Contributor Author

BoPeng commented Feb 8, 2017

What solution? (I did not read through the ticket). You can patch sos_executor.py in whatever way you like.

@gaow
Copy link
Member

gaow commented Feb 8, 2017

The solution is the last post of the ticket leave=None but I just tried it out. It apparently is not implemented yet. It looks like a proposed interface from 5 days ago that leave = None translates to leave = pos == 0 in order to clear the last line there. I think we can wait for the upstream change.

@BoPeng
Copy link
Contributor Author

BoPeng commented Feb 8, 2017

Ok. Let us keep this ticket open and wait for their resolution.

@BoPeng
Copy link
Contributor Author

BoPeng commented Jul 10, 2017

No progress after a few months.

@BoPeng BoPeng closed this as completed Jul 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants