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

schedulers,cli: persist newline breaks in log_iter #425

Closed
wants to merge 1 commit into from
Closed

Conversation

d4l3k
Copy link
Member

@d4l3k d4l3k commented Mar 15, 2022

This resolves #424

This makes it so the torchx scheduler log_iter method keeps the line breaks so downstream log streams can handle them gracefully. The current solution strips all \n characters and always adds them so it makes it impossible to do streaming visualizations of progress bars which use \r without a new line break.

WARNING: This is a change in the log_iter interface and all schedulers/downstream consumers will need to be updated.

If someone is logging from multiple workers this gets dangerous since the progress bar \r lines can clobber each other.

Test plan:

(torchx-3.10.2) tristanr@tristanr-arch2 ~/D/torchx-proj> torchx run --scheduler local_docker --wait --log utils.python --script test_tqdm.py
torchx 2022-03-15 14:26:42 INFO     loaded configs from /home/tristanr/Developer/torchx-proj/.torchxconfig
torchx 2022-03-15 14:26:42 INFO     Building workspace: file:///home/tristanr/Developer/torchx-proj for role[0]: python, image: ghcr.io/pytorch/torchx:0.1.2
dev0
torchx 2022-03-15 14:26:43 INFO     Done building workspace
torchx 2022-03-15 14:26:43 INFO     New image: sha256:9cfaf70f7143b4caef383b46c23635eaf001cbd3d9ff55335aa1ff8c5e236388 built from workspace
local_docker://torchx/torchx_utils_python-bprr9rb4k764nd
torchx 2022-03-15 14:26:44 INFO     Waiting for the app to finish...
python/0 100%|██████████| 100/100 [00:03<00:00, 32.95it/s]
torchx 2022-03-15 14:26:48 INFO     Job finished: SUCCEEDED
(torchx-3.10.2) tristanr@tristanr-arch2 ~/D/torchx-proj> torchx run --scheduler local_cwd --wait --log utils.python --script test_tqdm.py
torchx 2022-03-15 14:26:52 INFO     loaded configs from /home/tristanr/Developer/torchx-proj/.torchxconfig
torchx 2022-03-15 14:26:52 INFO     Log files located in: /tmp/torchx_0nqvqm1d/torchx/torchx_utils_python-x217jjqhbkkrgd/python/0
local_cwd://torchx/torchx_utils_python-x217jjqhbkkrgd
torchx 2022-03-15 14:26:52 INFO     Waiting for the app to finish...
python/0 100%|██████████| 100/100 [00:03<00:00, 32.95it/s]
torchx 2022-03-15 14:26:56 INFO     Job finished: SUCCEEDED

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 15, 2022
@facebook-github-bot
Copy link
Contributor

@d4l3k has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

❗ No coverage uploaded for pull request base (main@74560bb). Click here to learn what that means.
The diff coverage is 95.65%.

@@           Coverage Diff           @@
##             main     #425   +/-   ##
=======================================
  Coverage        ?   94.52%           
=======================================
  Files           ?       64           
  Lines           ?     3782           
  Branches        ?        0           
=======================================
  Hits            ?     3575           
  Misses          ?      207           
  Partials        ?        0           
Impacted Files Coverage Δ
torchx/runner/api.py 96.57% <ø> (ø)
torchx/cli/cmd_log.py 94.50% <87.50%> (ø)
torchx/schedulers/api.py 94.04% <100.00%> (ø)
torchx/schedulers/aws_batch_scheduler.py 87.62% <100.00%> (ø)
torchx/schedulers/docker_scheduler.py 95.69% <100.00%> (ø)
torchx/schedulers/kubernetes_scheduler.py 92.68% <100.00%> (ø)
torchx/schedulers/local_scheduler.py 92.93% <100.00%> (ø)
torchx/schedulers/ray_scheduler.py 96.21% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74560bb...0ab00b2. Read the comment docs.

d4l3k added a commit that referenced this pull request Mar 22, 2022
Summary:
This resolves #424

This makes it so the torchx scheduler `log_iter` method keeps the line breaks so downstream log streams can handle them gracefully. The current solution strips all `\n` characters and always adds them so it makes it impossible to do streaming visualizations of progress bars which use `\r` without a new line break.

WARNING: This is a change in the log_iter interface and all schedulers/downstream consumers will need to be updated.

If someone is logging from multiple workers this gets dangerous since the progress bar `\r` lines can clobber each other.

Pull Request resolved: #425

Test Plan:
(torchx-3.10.2) tristanr@tristanr-arch2 ~/D/torchx-proj> torchx run --scheduler local_docker --wait --log utils.python --script test_tqdm.py
torchx 2022-03-15 14:26:42 INFO     loaded configs from /home/tristanr/Developer/torchx-proj/.torchxconfig
torchx 2022-03-15 14:26:42 INFO     Building workspace: file:///home/tristanr/Developer/torchx-proj for role[0]: python, image: ghcr.io/pytorch/torchx:0.1.2
dev0
torchx 2022-03-15 14:26:43 INFO     Done building workspace
torchx 2022-03-15 14:26:43 INFO     New image: sha256:9cfaf70f7143b4caef383b46c23635eaf001cbd3d9ff55335aa1ff8c5e236388 built from workspace
local_docker://torchx/torchx_utils_python-bprr9rb4k764nd
torchx 2022-03-15 14:26:44 INFO     Waiting for the app to finish...
python/0 100%|██████████| 100/100 [00:03<00:00, 32.95it/s]
torchx 2022-03-15 14:26:48 INFO     Job finished: SUCCEEDED
(torchx-3.10.2) tristanr@tristanr-arch2 ~/D/torchx-proj> torchx run --scheduler local_cwd --wait --log utils.python --script test_tqdm.py
torchx 2022-03-15 14:26:52 INFO     loaded configs from /home/tristanr/Developer/torchx-proj/.torchxconfig
torchx 2022-03-15 14:26:52 INFO     Log files located in: /tmp/torchx_0nqvqm1d/torchx/torchx_utils_python-x217jjqhbkkrgd/python/0
local_cwd://torchx/torchx_utils_python-x217jjqhbkkrgd
torchx 2022-03-15 14:26:52 INFO     Waiting for the app to finish...
python/0 100%|██████████| 100/100 [00:03<00:00, 32.95it/s]
torchx 2022-03-15 14:26:56 INFO     Job finished: SUCCEEDED

Differential Revision: D34907682

Pulled By: d4l3k

fbshipit-source-id: 2c2619b05366074870434444acf1e0b02787fb77
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34907682

d4l3k added a commit that referenced this pull request Mar 23, 2022
Summary:
This resolves #424

This makes it so the torchx scheduler `log_iter` method keeps the line breaks so downstream log streams can handle them gracefully. The current solution strips all `\n` characters and always adds them so it makes it impossible to do streaming visualizations of progress bars which use `\r` without a new line break.

WARNING: This is a change in the log_iter interface and all schedulers/downstream consumers will need to be updated.

If someone is logging from multiple workers this gets dangerous since the progress bar `\r` lines can clobber each other.

Pull Request resolved: #425

Test Plan:
(torchx-3.10.2) tristanr@tristanr-arch2 ~/D/torchx-proj> torchx run --scheduler local_docker --wait --log utils.python --script test_tqdm.py
torchx 2022-03-15 14:26:42 INFO     loaded configs from /home/tristanr/Developer/torchx-proj/.torchxconfig
torchx 2022-03-15 14:26:42 INFO     Building workspace: file:///home/tristanr/Developer/torchx-proj for role[0]: python, image: ghcr.io/pytorch/torchx:0.1.2
dev0
torchx 2022-03-15 14:26:43 INFO     Done building workspace
torchx 2022-03-15 14:26:43 INFO     New image: sha256:9cfaf70f7143b4caef383b46c23635eaf001cbd3d9ff55335aa1ff8c5e236388 built from workspace
local_docker://torchx/torchx_utils_python-bprr9rb4k764nd
torchx 2022-03-15 14:26:44 INFO     Waiting for the app to finish...
python/0 100%|██████████| 100/100 [00:03<00:00, 32.95it/s]
torchx 2022-03-15 14:26:48 INFO     Job finished: SUCCEEDED
(torchx-3.10.2) tristanr@tristanr-arch2 ~/D/torchx-proj> torchx run --scheduler local_cwd --wait --log utils.python --script test_tqdm.py
torchx 2022-03-15 14:26:52 INFO     loaded configs from /home/tristanr/Developer/torchx-proj/.torchxconfig
torchx 2022-03-15 14:26:52 INFO     Log files located in: /tmp/torchx_0nqvqm1d/torchx/torchx_utils_python-x217jjqhbkkrgd/python/0
local_cwd://torchx/torchx_utils_python-x217jjqhbkkrgd
torchx 2022-03-15 14:26:52 INFO     Waiting for the app to finish...
python/0 100%|██████████| 100/100 [00:03<00:00, 32.95it/s]
torchx 2022-03-15 14:26:56 INFO     Job finished: SUCCEEDED

Reviewed By: kiukchung

Differential Revision: D34907682

Pulled By: d4l3k

fbshipit-source-id: 1750a301ca7c8319df4fbeca35fa8eb29b8ecaae
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34907682

Summary:
This resolves #424

This makes it so the torchx scheduler `log_iter` method keeps the line breaks so downstream log streams can handle them gracefully. The current solution strips all `\n` characters and always adds them so it makes it impossible to do streaming visualizations of progress bars which use `\r` without a new line break.

WARNING: This is a change in the log_iter interface and all schedulers/downstream consumers will need to be updated.

If someone is logging from multiple workers this gets dangerous since the progress bar `\r` lines can clobber each other.

Pull Request resolved: #425

Test Plan:
(torchx-3.10.2) tristanr@tristanr-arch2 ~/D/torchx-proj> torchx run --scheduler local_docker --wait --log utils.python --script test_tqdm.py
torchx 2022-03-15 14:26:42 INFO     loaded configs from /home/tristanr/Developer/torchx-proj/.torchxconfig
torchx 2022-03-15 14:26:42 INFO     Building workspace: file:///home/tristanr/Developer/torchx-proj for role[0]: python, image: ghcr.io/pytorch/torchx:0.1.2
dev0
torchx 2022-03-15 14:26:43 INFO     Done building workspace
torchx 2022-03-15 14:26:43 INFO     New image: sha256:9cfaf70f7143b4caef383b46c23635eaf001cbd3d9ff55335aa1ff8c5e236388 built from workspace
local_docker://torchx/torchx_utils_python-bprr9rb4k764nd
torchx 2022-03-15 14:26:44 INFO     Waiting for the app to finish...
python/0 100%|██████████| 100/100 [00:03<00:00, 32.95it/s]
torchx 2022-03-15 14:26:48 INFO     Job finished: SUCCEEDED
(torchx-3.10.2) tristanr@tristanr-arch2 ~/D/torchx-proj> torchx run --scheduler local_cwd --wait --log utils.python --script test_tqdm.py
torchx 2022-03-15 14:26:52 INFO     loaded configs from /home/tristanr/Developer/torchx-proj/.torchxconfig
torchx 2022-03-15 14:26:52 INFO     Log files located in: /tmp/torchx_0nqvqm1d/torchx/torchx_utils_python-x217jjqhbkkrgd/python/0
local_cwd://torchx/torchx_utils_python-x217jjqhbkkrgd
torchx 2022-03-15 14:26:52 INFO     Waiting for the app to finish...
python/0 100%|██████████| 100/100 [00:03<00:00, 32.95it/s]
torchx 2022-03-15 14:26:56 INFO     Job finished: SUCCEEDED

Reviewed By: kiukchung

Differential Revision: D34907682

Pulled By: d4l3k

fbshipit-source-id: c87f7931be3d90cf006080d3cb1e54b17016e930
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34907682

@d4l3k d4l3k deleted the lognewlines branch April 13, 2022 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gracefully handle \r carriage returns to support progress bars
2 participants