Skip to content

Commit a6ec7d7

Browse files
committed
Update requirements from discussion
1 parent ff90402 commit a6ec7d7

File tree

1 file changed

+25
-8
lines changed

1 file changed

+25
-8
lines changed

RFC-0026-logging-system.md

+25-8
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ Create a message logging system for PyTorch with the following requirements:
3838
- **Prototype**: Emitted when a prototype feature is called. See
3939
[PyTorch feature classifications](https://pytorch.org/blog/pytorch-feature-classification-changes/)
4040

41+
- TODO: Should all the classic Python errors and warnings (`TypeError`,
42+
`ValueError`, `NotImplementedError`, `DeprecationWarning`, etc) have their
43+
own message class? Or are those separate from our concept of a message
44+
class, and any message class is allowed to raise any Python exception or
45+
warning type?
46+
4147
* Continue using warning/error APIs that currently exist in PyTorch wherever
4248
possible. For instance, `TORCH_CHECK`, `TORCH_WARN`, and `TORCH_WARN_ONCE`
4349
should continue to be used in C++
@@ -47,30 +53,41 @@ Create a message logging system for PyTorch with the following requirements:
4753

4854
* Creating new message classes and severity levels should be easy
4955

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

5261
* Settings to disable specific message classes and severity levels
5362

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

69+
- Disabling warnings in Python should already be possible with the `warnings`
70+
module filter. See [documentation](https://docs.python.org/3/library/warnings.html#the-warnings-filter).
71+
There is no similar system in C++ at the moment, and building one is probably
72+
low priority.
73+
74+
- Filtering out **Info** severity messages would also be nice to have, since
75+
excessive printouts can degrade the user experience. Related to
76+
issue [#68768](https://github.com/pytorch/pytorch/issues/68768)
77+
6078
* Settings to avoid emitting duplicate messages generated by multiple
61-
`torch.distribted` ranks (related to issue
62-
[#68768](https://github.com/pytorch/pytorch/issues/68768))
79+
`torch.distributed` ranks. Related to issue
80+
[#68768](https://github.com/pytorch/pytorch/issues/68768)
6381

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

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

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

72-
- TODO: Should warn-once be its own separate severity level?
73-
7491
* Settings can be changed from Python, C++, or environment variables
7592

7693
- Filtering warnings with Python command line arguments should

0 commit comments

Comments
 (0)