Skip to content

Commit

Permalink
More updates from discussion
Browse files Browse the repository at this point in the history
  • Loading branch information
kurtamohler committed Aug 3, 2022
1 parent d272df6 commit c769f13
Showing 1 changed file with 32 additions and 27 deletions.
59 changes: 32 additions & 27 deletions RFC-0026-logging-system.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ Create a message logging system for PyTorch with the following requirements:
- **Error**: Emits a message as an error. If an error is never caught, the
application will print the error to stderr and quit.

- TODO: Do we also need a **Fatal** severity for integration with Meta's
internal logging system (glog)? A fatal message terminates the program

* Offer different message classes under each severity level.

- Every message is emitted as an instance of a message class.
Expand All @@ -48,35 +45,34 @@ Create a message logging system for PyTorch with the following requirements:
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: Error classes should never be disableable, right?
* Settings to disable specific **Warning** or **Info** classes

- Disabling warnings in Python is already 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)
- Filtering out **Info** messages would be nice to have because excessive
printouts can degrade the user experience. Related to issue
[#68768](https://github.com/pytorch/pytorch/issues/68768)

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

* Ability to make a particular warning only warn once. Warn-once should be the
default for most warnings.
* Ability to make a particular **Warning** or **Info** message only emit once.
Warn-once should be the default for most warnings.

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

- Offer a filter to override warn- and log-once, so that they always emit.
The filter could work similarly to the Python `warnings` filter. This is
a low priority feature.

- TODO: `torch.set_warn_always()` currently controls some warnings (maybe
only the ones from C++? I need to find out for sure.)

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

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

- Filtering warnings with Python command line arguments should
Expand Down Expand Up @@ -170,10 +166,14 @@ exists in C++, and it is not implemented as a C++ class that can be inherited
* **`c10::BetaWarning`** - Python `torch.BetaWarning`
- Emitted when a beta feature is called. See
[PyTorch feature classifications](https://pytorch.org/blog/pytorch-feature-classification-changes/).
- TODO: This warning type might not be very useful--find out if we really
want this

* **`c10::PrototypeWarning`** - Python `torch.PrototypeWarning`
- Emitted when a prototype feature is called. See
[PyTorch feature classifications](https://pytorch.org/blog/pytorch-feature-classification-changes/).
- TODO: This warning type might not be very useful--find out if we really
want this

* **`c10::NondeterministicWarning`** - Python `torch.NondeterministicWarning`
- Emitted when `torch.use_deterministic_algorithms(True)` and
Expand All @@ -198,9 +198,18 @@ In order to emit messages, developers can use the APIs defined in this section.

These APIs all have a variable length argument list, `...` in C++ and `*args`
in Python. When a message is emitted, these arguments are concatenated into
a string, and the string becomes the body of the message. In C++, the arguments
must all have the `std::ostream& operator<<` function defined so that they can
be concatenated, and in Python, they must all have a `__str__` function.
a string, and the string becomes the body of the message.

In C++, the arguments in `...` must all have the `std::ostream& operator<<`
function defined so that they can be concatenated.

In Python, each element in `*args` must either have a `__str__` function or it
must be a callable that, when called, produces another object that has
a `__str__` fuction. Providing the body of a message as a callable can provide
better performance in cases where the message would not be emitted, as in
`torch.check(True, lambda: expensive_function())` if `cond == True`, since the
`expensive_function()` would not be called in that case.


#### Error APIs

Expand Down Expand Up @@ -335,13 +344,6 @@ TODO: Should we have a `TOCH_WARN_RANK` (and others) in C++ as well? Is there
an existing use case for it?


### Other details

At the moment in PyTorch, the Python `warnings` module is being publicly
included in `torch` as `torch.warnings`. This should probably be removed or
renamed to `_warnings` to avoid confusion.


# PyTorch's current messaging API

The rest of this document contains details about the current messaging API in
Expand Down Expand Up @@ -414,6 +416,7 @@ Python error class:

| C++ error class | Python error class |
| ------------------------------- | -------------------------- |
| `std::exception` | `RuntimeError` |
| `c10::Error` | `RuntimeError` |
| `c10::IndexError` | `IndexError` |
| `c10::ValueError` | `ValueError` |
Expand Down Expand Up @@ -446,13 +449,15 @@ message using `operator<<`.
`c10::Error` and its subclasses are translated into their corresponding Python
errors [in `CATCH_CORE_ERRORS`](https://github.com/pytorch/pytorch/blob/72e4aab74b927c1ba5c3963cb17b4c0dce6e56bf/torch/csrc/Exceptions.h#L54-L100).

However, not all of the `c10::Error` subclasses in the table above appear here.
I'm not sure yet what's up with that.
However, not all of the `c10::Error` subclasses in the table above appear here,
which could just be an oversight.

`CATCH_CORE_ERRORS` is included within the `END_HANDLE_TH_ERRORS` macro that
every Python-bound C++ function uses for handling errors. For instance,
most Python-bound C++ functions use for handling errors. For instance,
`THPVariable__is_view` uses the error handling macro
[here](https://github.com/pytorch/pytorch/blob/72e4aab74b927c1ba5c3963cb17b4c0dce6e56bf/tools/autograd/templates/python_variable_methods.cpp#L76).
There is also a similar `END_HANDLE_TH_ERRORS_PYBIND` macro that is used for
pybind-based bindings.


#### `torch::PyTorchError`
Expand Down

0 comments on commit c769f13

Please sign in to comment.