Audit AT_ASSERT sites in TensorImpl.h; doc improvements (#20649)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/20649
I went through every occurrence of AT_ASSERT in this file and
thought about whether or not it should be TORCH_INTERNAL_ASSERT
or TORCH_CHECK. I think I did a good job at it. Some thoughts:
- In order to decide if a check is "internal" or not, we must
think about where the separation between userspace and our internals
are. I think any code that utilizes the PyTorch or Caffe2 C++ frontends
count as userspace. An important collorary is that the majority of operator
code "counts" as userspace, even though it lives in our repository. This
is inline with TCB (trusted computing base) thinking: you want the TCB to
be as small as possible, and because we have a *lot* of operator
implementations, they should not count as TCB.
- The primary test I applied when considering an AT_ASSERT was whether or
not I could trigger this error by just making method calls on caffe2::Tensor
or at::Tensor. If I could, that made it a TORCH_CHECK. This covers most
of the misapplications of TORCH_INTERNAL_ASSERT. One place I didn't
do this was the "is variable" checks; I think you have to work a bit
harder to trigger this case, and userspace code is not mixing up
Variables and Tensros.
- I updated the docs for device_opt_, explaining when it could be nullopt.
(The nullopt checks here are TORCH_CHECK, because you can trigger them
by taking an undefined tensor and poking the methods.)
Differential Revision: D15395576
fbshipit-source-id: 1c51b396012e7d949fbb4258092cf80e5e6f851b
