-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Allow setting logger output format with TORCH_LOGS_FORMAT #111770
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
Conversation
TORCH_LOGS_FORMAT="%(levelname)s: %(message)s" will only dump output level and message contents [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/111770
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit bef7a8b with merge base 2b2b6ca ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
TORCH_LOGS_FORMAT="%(levelname)s: %(message)s" will only dump output level and message contents [ghstack-poisoned]
| if fmt is None: | ||
| return TorchLogsFormatter() | ||
| else: | ||
| return logging.Formatter(fmt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the formatter is invalid, what kind of error do we get?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we throw a ValueError: Formatting field not found in record error. We could find a way to validate it but not sure how valuable/easy it would be.
|
I get the desire to want to make it general purpose, but it really just seems like you want all annotations, or none (just the message). This would make it much simpler to understand, and harder to set wrong. |
|
@voznesenskym This is customizable enough to appease all different opinions, which is why i went with this version. There was not a consensus around simpler versions. Since this is not a publicly announced API, it should be okay to be less precise/hardened around it. What do you think? |
|
I wonder whether we should turn the default to not have that info |
|
@Chillee I proposed that but @voznesenskym was against it. It will probably be hard to get consensus on something like this. |
TORCH_LOGS_FORMAT="%(levelname)s: %(message)s" will only dump output level and message contents [ghstack-poisoned]
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Pull Request resolved: #111808 Approved by: https://github.com/Chillee ghstack dependencies: #111770
…f arguments (#111939) Pull Request resolved: #111939 Approved by: https://github.com/jansel, https://github.com/zou3519 ghstack dependencies: #111770, #111808
…1770) TORCH_LOGS_FORMAT="%(levelname)s: %(message)s" will only dump output level and message contents Pull Request resolved: pytorch#111770 Approved by: https://github.com/jansel
Pull Request resolved: pytorch#111808 Approved by: https://github.com/Chillee ghstack dependencies: pytorch#111770
…f arguments (pytorch#111939) Pull Request resolved: pytorch#111939 Approved by: https://github.com/jansel, https://github.com/zou3519 ghstack dependencies: pytorch#111770, pytorch#111808
…1770) TORCH_LOGS_FORMAT="%(levelname)s: %(message)s" will only dump output level and message contents Pull Request resolved: pytorch#111770 Approved by: https://github.com/jansel
Pull Request resolved: pytorch#111808 Approved by: https://github.com/Chillee ghstack dependencies: pytorch#111770
…f arguments (pytorch#111939) Pull Request resolved: pytorch#111939 Approved by: https://github.com/jansel, https://github.com/zou3519 ghstack dependencies: pytorch#111770, pytorch#111808
Stack from ghstack (oldest at bottom):
TORCH_LOGS_FORMAT="%(levelname)s: %(message)s" will only dump output
level and message contents