pytorch
6f543e0d - add not_close_error_metas for internal comparison machinery (#90004)

Commit
1 year ago
add not_close_error_metas for internal comparison machinery (#90004) While discussing a possible addition of `assert_not_close` to the API (See #90005 later in the stack), it became clear that we should have an intermediate function that returns a bool-ish value that one can assert on. This PR introduces this function as `are_equal` as replacement for `assert_equal`. Interface is the same, but instead of raising in case a comparison failed, we return the `ErrorMeta`'s of all failures and leave it to the caller to handle. Note that this only applies to errors raised during the comparison stage. Everything else, e.g. only setting `atol` *or* `rtol`, will raise just as before. We decided to keep this private for now unless there is user demand. The largest issue that needs to be solved before this can become public is the return type: if we have something like `torch.testing.are_close` we are targeting two uses cases: 1. Using it to branch inside code like `if are_close(...):` 2. Using it to assert closeness inside a test like `assert are_close(...)`. This is the default way to assert something with `pytest` To do that, the return type has to be bool-ish, i.e. being an instance of `bool` or implementing `__bool__`. Plus, `bool(are_close()) is True` needs to be the if the inputs are close and `False` otherwise. The current logic of `are_close` satisfies the former, but violates the latter. In case everything is close, we return an empty list, but `bool([]) is False`. Directly using an instance of `bool` would work for the requirements above, but then we would have no option to add diagnositics to the error. Meaning `assert are_close()` would work, but would be non-descriptive. Using `Tuple[bool, str]` would work in general, but is quite dangerous and unexpected: since all non-empty tuples evaluate to `True`, this can easily hide bugs if the user is not super careful: ```pycon >>> close = (False, "error message with diagnostics") >>> assert close[0] AssertionError: error message with diagnostics >>> assert close ``` One possible solution here would be a thin custom object: ```py class Close: def __init__(self, flag:bool, msg: str = "") -> None: self._flag = flag self._msg = msg def __bool__(self): return self._flag def __str__(self): return self._msg ``` Now we can do something like ```pycon close = Close(False, "error message with diagnostics") # coming from are_close >>> if not close: ... print("It works!") It works! >>> assert close AssertionError >>> assert close, close # This looks weird, but does its job AssertionError: error message with diagnostics ``` But this means we introduce another abstraction that the user has to deal with. To reiterate, we are not going to make `are_close` public until there is user demand, since none of the options above is without flaws. Pull Request resolved: https://github.com/pytorch/pytorch/pull/90004 Approved by: https://github.com/mruberry, https://github.com/malfet
Author
Committer
Parents
Loading