[ONNX] Migrate to PT2 logging #106592

BowenBao · 2023-08-04T00:57:09Z

Stack from ghstack (oldest at bottom):

Summary

The 'dynamo_export' diagnostics leverages the PT2 artifact logger to handle the verbosity
level of logs that are recorded in each SARIF log diagnostic. In addition to SARIF log,
terminal logging is by default disabled. Terminal logging can be activated by setting
the environment variable TORCH_LOGS="onnx_diagnostics". When the environment variable
is set, it also fixes logging level to logging.DEBUG, overriding the verbosity level
specified in the diagnostic options.
See torch/_logging/__init__.py for more on PT2 logging.
Replaces 'with_additional_message' with 'Logger.log' like apis.
Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip
evaluation if the message will not be logged into diagnostic.
Introduce 'log_source_exception' for easier exception logging.
Introduce 'log_section' for easier markdown title logging.
Updated all existing code to use new api.
Removed 'arg_format_too_verbose' diagnostic.
Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid
confusion.

Follow ups

The 'dynamo_export' diagnostic now will not capture python stack
information at point of diagnostic creation. This will be added back in
follow up PRs for debug level logging.
There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext'
for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will
attempt to fix it.
More docstrings with examples.

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. [ghstack-poisoned]

pytorch-bot · 2023-08-04T00:57:11Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/106592

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ 3 Unrelated Failures

As of commit 0e223a3:

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. ghstack-source-id: cb731ec7ee3957ff26367ff52419766de6fa9b65 Pull Request resolved: #106592

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. - Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. [ghstack-poisoned]

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. ghstack-source-id: 4cc785e10fb4a9eccf9b62e7f9540cb5aa2bc0e8 Pull Request resolved: #106592

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. - Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. [ghstack-poisoned]

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. ghstack-source-id: 088f441b5f2f20c4a983194b7fb35c389a29582d Pull Request resolved: #106592

Summary - The 'dynamo_export' diagnostics leverages the PT2 artifact logger to handle the verbosity level of logs that are recorded in each SARIF log diagnostic. In addition to SARIF log, terminal logging is by default disabled. Terminal logging can be activated by setting the environment variable `TORCH_LOGS="onnx_diagnostics"`. When the environment variable is set, it also fixes logging level to `logging.DEBUG`, overriding the verbosity level specified in the diagnostic options. See `torch/_logging/__init__.py` for more on PT2 logging. - Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. [ghstack-poisoned]

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. ghstack-source-id: b979765ade83f6372095cfc81708a31dc8984dca Pull Request resolved: #106592

Summary - The 'dynamo_export' diagnostics leverages the PT2 artifact logger to handle the verbosity level of logs that are recorded in each SARIF log diagnostic. In addition to SARIF log, terminal logging is by default disabled. Terminal logging can be activated by setting the environment variable `TORCH_LOGS="onnx_diagnostics"`. When the environment variable is set, it also fixes logging level to `logging.DEBUG`, overriding the verbosity level specified in the diagnostic options. See `torch/_logging/__init__.py` for more on PT2 logging. - Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. [ghstack-poisoned]

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. ghstack-source-id: 9cac69b43b356d66852c8511a966203db9ea0bd1 Pull Request resolved: #106592

torch/onnx/_internal/diagnostics/infra/context.py

torch/onnx/_internal/diagnostics/infra/formatter.py

torch/onnx/_internal/diagnostics/infra/_infra.py

Summary - The 'dynamo_export' diagnostics leverages the PT2 artifact logger to handle the verbosity level of logs that are recorded in each SARIF log diagnostic. In addition to SARIF log, terminal logging is by default disabled. Terminal logging can be activated by setting the environment variable `TORCH_LOGS="onnx_diagnostics"`. When the environment variable is set, it also fixes logging level to `logging.DEBUG`, overriding the verbosity level specified in the diagnostic options. See `torch/_logging/__init__.py` for more on PT2 logging. - Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. [ghstack-poisoned]

titaiwangms · 2023-08-08T18:50:35Z

Can #106478 be merged before this PR lol

BowenBao · 2023-08-08T20:39:50Z

Can #106478 be merged before this PR lol

Sure, yours is earlier :)

titaiwangms

Is diagnostics.levels.ERROR affected?

docs/source/onnx_diagnostics.rst

test/onnx/internal/test_diagnostics.py

torch/onnx/_internal/fx/passes/type_promotion.py

torch/onnx/_internal/fx/op_validation.py

torch/onnx/_internal/diagnostics/infra/context.py

torch/onnx/_internal/diagnostics/infra/formatter.py

torch/onnx/_internal/fx/op_validation.py

Summary - The 'dynamo_export' diagnostics leverages the PT2 artifact logger to handle the verbosity level of logs that are recorded in each SARIF log diagnostic. In addition to SARIF log, terminal logging is by default disabled. Terminal logging can be activated by setting the environment variable `TORCH_LOGS="onnx_diagnostics"`. When the environment variable is set, it also fixes logging level to `logging.DEBUG`, overriding the verbosity level specified in the diagnostic options. See `torch/_logging/__init__.py` for more on PT2 logging. - Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. [ghstack-poisoned]

thiagocrepaldi

Going OOF, so I will defer this to our colleagues. :)

The only thing I spotted is having a global logger instead of part of Diagnostic classes

thiagocrepaldi · 2023-08-11T18:55:42Z

torch/onnx/_internal/diagnostics/infra/context.py

@@ -16,6 +18,7 @@

 # This is a workaround for mypy not supporting Self from typing_extensions.
 _Diagnostic = TypeVar("_Diagnostic", bound="Diagnostic")
+diagnostic_logger: logging.Logger = logging.getLogger(__name__)


Is it possible to move to the diagnostic ctor? It is nicer to not depend on globals as much as possible

Similar discussion here #106741 (comment) on idiomatic global logger.

It is a bit anti-pattern that Diagnostic needs to keep track of logger, but I feel it is better to keep track of the global logger instead of a global var of __name__ value here to retrieve that same logger.

BowenBao · 2023-08-11T20:53:33Z

@pytorchbot merge

BowenBao · 2023-08-11T20:54:02Z

Merging but will address any more comments in follow ups.

pytorchmergebot · 2023-08-11T20:55:05Z

Merge started

Your 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

Advanced Debugging

Check the merge workflow status
here

Summary - 'dynamo_export' diagnostics now utilize PT2 artifact logger to manage the verbosity level of logs recorded into each diagnostic in SARIF log. The terminal logging is by default turned off. Setting environment variable 'TORCH_LOGS="+onnx_diagnostics"' turns on terminal logging, as well as adjusts the verbosity level which overrides that in diagnostic options. Replaces 'with_additional_message' with 'Logger.log' like apis. - Introduce 'LazyString', adopted from 'torch._dynamo.utils', to skip evaluation if the message will not be logged into diagnostic. - Introduce 'log_source_exception' for easier exception logging. - Introduce 'log_section' for easier markdown title logging. - Updated all existing code to use new api. - Removed 'arg_format_too_verbose' diagnostic. - Rename legacy diagnostic classes for TorchScript Onnx Exporter to avoid confusion. Follow ups - The 'dynamo_export' diagnostic now will not capture python stack information at point of diagnostic creation. This will be added back in follow up PRs for debug level logging. - There is type mismatch due to subclassing 'Diagnostic' and 'DiagnosticContext' for 'dynamo_export' to incorporate with PT2 logging. Follow up PR will attempt to fix it. - More docstrings with examples. ghstack-source-id: fd5ad1132f3a71c56a3d16c6c693ee088cdcc185 Pull Request resolved: pytorch/pytorch#106592

BowenBao mentioned this pull request Aug 4, 2023

[ONNX] Remove legacy diagnostic printing #106498

Closed

pytorch-bot bot added the release notes: onnx torch.onnx related changes that should show up in the release notes label Aug 4, 2023

pytorchbot added the open source label Aug 4, 2023

BowenBao added the topic: improvements topic category label Aug 4, 2023

BowenBao marked this pull request as ready for review August 5, 2023 00:08

BowenBao requested review from abock and thiagocrepaldi as code owners August 5, 2023 00:08

thiagocrepaldi reviewed Aug 7, 2023

View reviewed changes

BowenBao mentioned this pull request Aug 8, 2023

[ONNX] Public diagnostic options for 'dynamo_export' #106741

Closed

titaiwangms self-requested a review August 8, 2023 18:48

titaiwangms added the module: onnx Related to torch.onnx label Aug 8, 2023

titaiwangms reviewed Aug 9, 2023

View reviewed changes

BowenBao added 2 commits August 10, 2023 14:14

titaiwangms approved these changes Aug 11, 2023

View reviewed changes

thiagocrepaldi reviewed Aug 11, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 11, 2023

pytorchmergebot added the merging label Aug 11, 2023

pytorchmergebot added Merged and removed merging labels Aug 11, 2023

pytorchmergebot closed this in 22095ac Aug 11, 2023

facebook-github-bot deleted the gh/BowenBao/283/head branch August 15, 2023 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ONNX] Migrate to PT2 logging #106592

[ONNX] Migrate to PT2 logging #106592

BowenBao commented Aug 4, 2023 •

edited

pytorch-bot bot commented Aug 4, 2023 •

edited

titaiwangms commented Aug 8, 2023

BowenBao commented Aug 8, 2023

titaiwangms left a comment

thiagocrepaldi left a comment

thiagocrepaldi Aug 11, 2023

BowenBao Aug 11, 2023

BowenBao commented Aug 11, 2023

BowenBao commented Aug 11, 2023

pytorchmergebot commented Aug 11, 2023

[ONNX] Migrate to PT2 logging #106592

[ONNX] Migrate to PT2 logging #106592

Conversation

BowenBao commented Aug 4, 2023 • edited

pytorch-bot bot commented Aug 4, 2023 • edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/106592

✅ 3 Unrelated Failures

titaiwangms commented Aug 8, 2023

BowenBao commented Aug 8, 2023

titaiwangms left a comment

Choose a reason for hiding this comment

thiagocrepaldi left a comment

Choose a reason for hiding this comment

thiagocrepaldi Aug 11, 2023

Choose a reason for hiding this comment

BowenBao Aug 11, 2023

Choose a reason for hiding this comment

BowenBao commented Aug 11, 2023

BowenBao commented Aug 11, 2023

pytorchmergebot commented Aug 11, 2023

Merge started

BowenBao commented Aug 4, 2023 •

edited

pytorch-bot bot commented Aug 4, 2023 •

edited