Skip to content

Commit

Permalink
Update requirements from discussion
Browse files Browse the repository at this point in the history
  • Loading branch information
kurtamohler committed Jul 5, 2022
1 parent ff90402 commit ba89c08
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
Binary file added .RFC-0000-template.md.swp
Binary file not shown.
33 changes: 25 additions & 8 deletions RFC-0026-logging-system.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ Create a message logging system for PyTorch with the following requirements:
- **Prototype**: Emitted when a prototype feature is called. See
[PyTorch feature classifications](https://pytorch.org/blog/pytorch-feature-classification-changes/)

- TODO: Should all the classic Python errors and warnings (`TypeError`,
`ValueError`, `NotImplementedError`, `DeprecationWarning`, etc) have their
own message class? Or are those separate from our concept of a message
class, and any message class is allowed to raise any Python exception or
warning type?

* Continue using warning/error APIs that currently exist in PyTorch wherever
possible. For instance, `TORCH_CHECK`, `TORCH_WARN`, and `TORCH_WARN_ONCE`
should continue to be used in C++
Expand All @@ -47,30 +53,41 @@ Create a message logging system for PyTorch with the following requirements:

* Creating new message classes and severity levels should be easy

* Settings to turn warnings for a specific message class into errors
* Ability to turn warnings into errors. This is already possible with the
Python `warnings` module filter, but the PyTorch docs should mention it and
we should probably have unit tests for it.
See [documentation](https://docs.python.org/3/library/warnings.html#the-warnings-filter)

* Settings to disable specific message classes and severity levels

- TODO: However, most errors should not be disableable, right? Perhaps only
- TODO: Most errors should not be disableable, right? Perhaps only
some message classes should allow disabling or downgrading errors. For
instance, currently in PyTorch, we can downgrade a nondeterministic error
to a warning, but we wouldn't want to downgrade an error from invalid
arguments given to an operation.

- Disabling warnings in Python should already be possible with the `warnings`
module filter. See [documentation](https://docs.python.org/3/library/warnings.html#the-warnings-filter).
There is no similar system in C++ at the moment, and building one is probably
low priority.

- Filtering out **Info** severity messages would also be nice to have, since
excessive printouts can degrade the user experience. Related to
issue [#68768](https://github.com/pytorch/pytorch/issues/68768)

* Settings to avoid emitting duplicate messages generated by multiple
`torch.distribted` ranks (related to issue
[#68768](https://github.com/pytorch/pytorch/issues/68768))
`torch.distributed` ranks. Related to issue
[#68768](https://github.com/pytorch/pytorch/issues/68768)

* Ability to make a particular warning only warn once
* Ability to make a particular warning only warn once. Warn-once should be the
default in most cases.

- NOTE: Currently `TORCH_WARN_ONCE` does this in C++, but there is no Python
equivalent

- TODO: Should there be a setting to turn a warn-always into a warn-once for
- TODO: Should there be a setting to turn a warn-once into a warn-always for
a given message class and vice versa?

- TODO: Should warn-once be its own separate severity level?

* Settings can be changed from Python, C++, or environment variables

- Filtering warnings with Python command line arguments should
Expand Down

0 comments on commit ba89c08

Please sign in to comment.