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

Monkepyatch TQDM Progress Bars when Streamed over the network #1237

Closed
wants to merge 3 commits into from

Conversation

ravi-mosaicml
Copy link
Contributor

@ravi-mosaicml ravi-mosaicml commented Jun 29, 2022

TQDM Progress Bars streamed over the network do not display until a \n is written. In effect, this caused the progress bars not to show, until they were finished. This behavior defeats the purpose of progress bars. This issue has been documented here: tqdm/tqdm#1319

This PR fixes this issue by attempting to detect whether we are in a K8S environment, and if so, then automatically write \n each time the progress bar is updated.

Example Output:

Downloading:   0%|          | 0.00/570 [00:00<?, ?B/s]
Downloading: 100%|██████████| 570/570 [00:00<00:00, 1.16MB/s]
Downloading:   0%|          | 0.00/28.0 [00:00<?, ?B/s]
Downloading: 100%|██████████| 28.0/28.0 [00:00<00:00, 52.7kB/s]
Downloading:   0%|          | 0.00/226k [00:00<?, ?B/s]
Downloading:  14%|█▍        | 32.0k/226k [00:00<00:00, 248kB/s]
Downloading:  83%|████████▎ | 188k/226k [00:00<00:00, 809kB/s] 
Downloading: 100%|██████████| 226k/226k [00:00<00:00, 867kB/s]
Downloading:   0%|          | 0.00/455k [00:00<?, ?B/s]
Downloading:  18%|█▊        | 84.0k/455k [00:00<00:00, 564kB/s]
Downloading:  80%|████████  | 365k/455k [00:00<00:00, 1.34MB/s]
Downloading: 100%|██████████| 455k/455k [00:00<00:00, 1.52MB/s]

TQDM Progress Bars streamed over the network do not display until a `\n` is written. In effect, this caused the progress bars not to show, until they were finished. This behavior defats the purpose of progress bars. This issue has been documented here: tqdm/tqdm#1319

This PR fixes this issue by attempting to detect whether we are in a K8S environment, and if so, then automatically write `\n` each time the progress bar is updated.
@ravi-mosaicml ravi-mosaicml marked this pull request as ready for review June 29, 2022 21:48
@moinnadeem
Copy link
Contributor

Wait, this just repeats the progress bar multiple times, right? I'm exploring if we should just use a FileLogger instead to solve the same problem with a better UX, and then explore a deeper solution next week.

Additionally, can we do sys.stdout.isatty() to detect whether we need to monkey patch? This would also create newlines when using interactive instances, or am I missing something?

@ravi-mosaicml
Copy link
Contributor Author

Wait, this just repeats the progress bar multiple times, right? I'm exploring if we should just use a FileLogger instead to solve the same problem with a better UX, and then explore a deeper solution next week.

Additionally, can we do sys.stdout.isatty() to detect whether we need to monkey patch? This would also create newlines when using interactive instances, or am I missing something?

Correct, this would repeat the progress bar multiple times. sys.stdout.isatty() is true when doing mcli run, so unfortunately cannot do that. Let me include an example output.

@ravi-mosaicml
Copy link
Contributor Author

Going to close this PR for now and see if we can fix the underlying issue with log streaming.

@Matthias-Mangin
Copy link

Hello @ravi-mosaicml , do you know if there are any updates on this ?

@hanlint
Copy link
Contributor

hanlint commented Oct 17, 2022

Hi @Matthias-Mangin , this was fixed in #1264, let me know if you have any issues there!

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.

None yet

4 participants