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

[ONNX] Diagnostic option 'warnings_as_errors' #105886

Closed
wants to merge 1 commit into from

Conversation

BowenBao
Copy link
Collaborator

@BowenBao BowenBao commented Jul 25, 2023

Stack from ghstack (oldest at bottom):

If set, diagnostics with level as WARNING will be logged as level
with ERROR, and immediately raised.

TODO: bikeshed public export api.

If set, diagnostics with level as WARNING will be logged as level
with ERROR, and immediately raised.

TODO: bikeshed public export api.

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Jul 25, 2023

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit e1ac710:
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added the release notes: onnx torch.onnx related changes that should show up in the release notes label Jul 25, 2023
BowenBao added a commit that referenced this pull request Jul 25, 2023
If set, diagnostics with level as WARNING will be logged as level
with ERROR, and immediately raised.

TODO: bikeshed public export api.

ghstack-source-id: 3684381362b7a7e78288be0aa189362a93b21cc0
Pull Request resolved: #105886
@BowenBao BowenBao added the topic: improvements topic category label Jul 25, 2023
diagnostic = diagnostic.with_source_exception(e)
self.context.log_and_raise_if_error(diagnostic)

def test_diagnostic_context_raises_if_diagnostic_is_warning_and_warnings_as_errors_is_true(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clear test naming 🎉

):
with self.assertRaises(infra.RuntimeErrorWithDiagnostic):
self.context.options.warnings_as_errors = True
self.context.log_and_raise_if_error(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to include this part to enable it? How do we use log_and_raise_if_error and option: warnings_as_errors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log_and_raise_if_error is used by diagnose_call internally, so for inflight diagnostics if you set its level as warning, and warnings_as_errors is turned on, the inflight diagnostics becomes an error and raises. E.g, if warnings_as_errors is turned on, failed op_level_debug w/ warning diagnostic will automatically become error and raise.

For regular diagnostics, one always need to log them into diagnostic_context. log_and_raise_if_error is the api for that.

warnings_as_errors is a flag under DiagnosticOptions. This is not exposed in export api though, that is left as a follow up to discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged open source release notes: onnx torch.onnx related changes that should show up in the release notes topic: improvements topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants