Skip to content

Revert "torchx/runner: log events to torch.monitor (#379)" #536

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

Closed
wants to merge 1 commit into from

Conversation

d4l3k
Copy link
Member

@d4l3k d4l3k commented Jun 24, 2022

This reverts commit 4b989d5.

Importing torch.monitor adds a pretty large dependency on torch which hugely slows down load time in OSS.

Test plan:

This isn't being used anywhere AFAIK so should be safe to land. Internal has a custom handler.

@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 Jun 24, 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 Jun 24, 2022

Codecov Report

Merging #536 (e88d2df) into main (3e739b9) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #536      +/-   ##
==========================================
+ Coverage   94.75%   94.81%   +0.06%     
==========================================
  Files          65       65              
  Lines        3949     3937      -12     
==========================================
- Hits         3742     3733       -9     
+ Misses        207      204       -3     
Impacted Files Coverage Δ
torchx/runner/events/__init__.py 96.66% <ø> (+5.00%) ⬆️
torchx/runner/events/api.py 66.66% <100.00%> (-2.57%) ⬇️

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 3e739b9...e88d2df. Read the comment docs.

@facebook-github-bot
Copy link
Contributor

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

d4l3k added a commit that referenced this pull request Jun 24, 2022
Summary:
This reverts commit 4b989d5.

Importing torch.monitor adds a pretty large dependency on torch which hugely slows down load time in OSS.

This accounts for about 340ms of load time.

Pull Request resolved: #536

Test Plan: This isn't being used anywhere AFAIK so should be safe to land. Internal has a custom handler.

Reviewed By: priyaramani, kurman

Differential Revision: D37432126

Pulled By: d4l3k

fbshipit-source-id: f38a89a7a4dbbd69e9ba43c130bd31b084c0bb00
Summary:
This reverts commit 4b989d5.

Importing torch.monitor adds a pretty large dependency on torch which hugely slows down load time in OSS.

This accounts for about 340ms of load time.

Pull Request resolved: #536

Test Plan: This isn't being used anywhere AFAIK so should be safe to land. Internal has a custom handler.

Reviewed By: priyaramani, kurman

Differential Revision: D37432126

Pulled By: d4l3k

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

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

@d4l3k d4l3k deleted the revertmonitor branch October 28, 2022 20:24
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.

3 participants