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

dvc push -q is not completely quiet, shows file transfer tqdm progress bars #3314

Closed
WillNichols726 opened this issue Feb 12, 2020 · 9 comments · Fixed by #3316
Closed

dvc push -q is not completely quiet, shows file transfer tqdm progress bars #3314

WillNichols726 opened this issue Feb 12, 2020 · 9 comments · Fixed by #3316
Assignees
Labels
bug Did we break something? p2-medium Medium priority, should be done, but less important refactoring Factoring and re-factoring research ui user interface / interaction

Comments

@WillNichols726
Copy link

Hey guys, love what you've done with DVC.

Had a quick bug that's causing me a little issue. When I use 'dvc push -q' I'm still seeing tqdm progress bars. Wouldn't be a huge issue, but I'm probably pushing 100K 250kb files. This is a local remote, so the transfer speeds are quick. I know in some of my other scripts where I use tqdm, if the iteration time is very small, the tqdm overhead of writing to std::out actually starts to contribute to performance.

dvc version: 0.83.0
os: Windows 10

image

@triage-new-issues triage-new-issues bot added the triage Needs to be triaged label Feb 12, 2020
@ghost ghost added bug Did we break something? ui user interface / interaction labels Feb 12, 2020
@triage-new-issues triage-new-issues bot removed triage Needs to be triaged labels Feb 12, 2020
@shcheklein
Copy link
Member

@casperdcl could you take a look please?

@casperdcl casperdcl self-assigned this Feb 12, 2020
@casperdcl
Copy link
Contributor

@WillNichols726

I know in some of my other scripts where I use tqdm, if the iteration time is very small, the tqdm overhead of writing to std::out actually starts to contribute to performance

That should really not be the case as tqdm is designed to be negligible overhead in all cases - could you elaborate a bit more? I know this isn't actually the issue here, though.

In terms of this issue, the progress output should be suppressed completely by -q. Will take a look & fix soon.

@casperdcl casperdcl added c1-quick-fix p2-medium Medium priority, should be done, but less important refactoring Factoring and re-factoring research labels Feb 12, 2020
@WillNichols726
Copy link
Author

@casperdcl - Appreciate everything you've done with tqdm! Recently, I had something that was running at 200 iterations a second, when I set mininterval to 1 sec, jumped up to 300 iterations a sec. Didn't spend a lot of time determining root cause for the speedup, it could've been caching behavior or something else. I've also seen some minor speedups on other rapidly-iterating scripts when increasing mininterval. Not a huge issue. I'm typically running in Powershell or cmd on Windows 10.

@casperdcl
Copy link
Contributor

Hmm interesting as the default mininterval=0.1 should be fine on most consoles.

No real measurable difference for me (it/s)... Note that it's under 200it/s because sleep isn't accurate.

$ python -c 'from time import sleep; from tqdm import trange
[sleep(1/200) for _ in trange(100)]'
100%|██████████| 100/100 [00:00<00:00, 192.77it/s]
$ python -c 'from time import sleep; from tqdm import trange
[sleep(1/200) for _ in trange(100, mininterval=1)]'
100%|█████████| 100/100 [00:00<00:00, 192.62it/s]

@casperdcl
Copy link
Contributor

ah also (back to this issue) --quiet is actually currently ignored (i.e. not implemented) by push

@casperdcl casperdcl added enhancement Enhances DVC feature request Requesting a new feature and removed bug Did we break something? c1-quick-fix labels Feb 12, 2020
@casperdcl
Copy link
Contributor

nvm looks like this is a bug

@casperdcl casperdcl added bug Did we break something? and removed enhancement Enhances DVC feature request Requesting a new feature labels Feb 12, 2020
casperdcl added a commit to casperdcl/dvc that referenced this issue Feb 12, 2020
efiop pushed a commit that referenced this issue Feb 14, 2020
* ui: default no_progress_bar=None instead of False

Fixes #3314

* ui: catch remaining missing auto-quiet progress

* progress: treat disable=False as None

* test: progress.Tqdm(disable=False)

* revert no_progress_bar=None <= False

partially reverts e0342a8
@efiop
Copy link
Member

efiop commented Feb 14, 2020

@WillNichols726 0.85.0 is out with this fix, could you please give it a try and let us know if it fixed it for you? 🙂

@efiop
Copy link
Member

efiop commented Feb 14, 2020

@WillNichols726 Btw, I suppose you were using -q because pbars were jumping around and being annoying? 🙂 If so, just wanted to let you know that we are working on a fix, so stay tuned for that too 🙂

@WillNichols726
Copy link
Author

@efiop - Yeah, was using -q to try and suppress the progress bars that were jumping around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Did we break something? p2-medium Medium priority, should be done, but less important refactoring Factoring and re-factoring research ui user interface / interaction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants