Repository: pytorch
Issue: Implement dunder getattribute instead of dunder getattr for nn.Module
Labels: module: bc-breaking,module: nn,triaged,topic: bc breaking

## 🚀 Feature

Implement `__getattribute__` instead of `__getattr__` for `nn.Module`.

## Motivation

The purpose behind this proposal is as a workaround to a Python issue that arises if a user employs the `@property` decorator in their `nn.Module` subclass. Lots of detail on the problem can be found in #33934 and #49726, as well as [here](https://stackoverflow.com/questions/25115366/property-and-getattr-compatibility-issue-with-attributeerror), [here](http://blog.devork.be/2011/06/using-getattr-and-property_17.html), and [here](https://medium.com/@ceshine/python-debugging-pitfall-mixed-use-of-property-and-getattr-f89e0ede13f1).

TL;DR: `__getattr__` and `@property` don't play well together, resulting in confusing error messages / poor debuggability for typos and some logic errors within `@property` methods.

Implementing `__getattr__`'s logic in `__getattribute__` would improve the UX of working with user-defined modules with `@property` methods. This is a common workaround; it is recommended [here](https://stackoverflow.com/questions/25115366/property-and-getattr-compatibility-issue-with-attributeerror) and it is present in at least one other open-source project: [Django](https://github.com/encode/django-rest-framework/pull/2530/files#diff-6dc9b019ec1d96c56e0e31dac3bba51cL488).

Quoting one of the original issue filers:
> I agree that this is in essence a Python issue. However, I still think that we should provide a workaround because this confusing phenomenon is really caused jointly by user and PyTorch (which defines custom `__getattr__`) and the users should not be expected to know this implementation detail, and thus may likely be confused (for instance, they wouldn't even know that they need to search "property and custom `__getattr__`" on google).

## Pitch

Essentially, move the logic currently in `__getattr__` directly into `__getattribute__`:

```python
def __getattribute__(self, name: str) -> Any:
    try:
        return super().__getattribute__(name)
    except AttributeError as e:
        if '_parameters' in self.__dict__:
            _parameters = self.__dict__['_parameters']
            if name in _parameters:
                return _parameters[name]
        if '_buffers' in self.__dict__:
            _buffers = self.__dict__['_buffers']
            if name in _buffers:
                return _buffers[name]
        if '_modules' in self.__dict__:
            modules = self.__dict__['_modules']
            if name in modules:
                return modules[name]

        # Note: Purposely reuse the specific error thrown from the original attempt to
        # access the property. This ensures that the error message is correct for the
        # case where a property method attempts to call a non-existent property, for example.
        raise e
```

## Alternatives

* [This PR](https://github.com/pytorch/pytorch/pull/34324) proposed an unsuccessful workaround that defines an `AttributeError` subclass named `ModuleAttributeError`.
* [This PR](https://github.com/pytorch/pytorch/pull/49879) proposed the following workaround, keeping the logic in `__getattr__`:
```python
def __getattr__(self, name: str) -> Union[Tensor, 'Module']:
    if '_parameters' in self.__dict__:
        _parameters = self.__dict__['_parameters']
        if name in _parameters:
            return _parameters[name]
    if '_buffers' in self.__dict__:
        _buffers = self.__dict__['_buffers']
        if name in _buffers:
            return _buffers[name]
    if '_modules' in self.__dict__:
        modules = self.__dict__['_modules']
        if name in modules:
            return modules[name]
    - raise ModuleAttributeError("'{}' object has no attribute '{}'".format(
    -    type(self).__name__, name))
    + super().__getattribute__(name)
```
That is, `__getattr__` calls back into `__getattribute__` instead of throwing. It seems to work, but relies on undocumented idiosyncrasies in Python's handling of `AttributeError`s and results in extra call overhead.

## Additional context

This issue exists to gather feedback on the impact that moving from `__getattr__` to `__getattribute__` in `nn.Module` would have. There's at least one area in PyTorch that currently depends on the implementation detail of `__getattr__` specifically being defined: [trace() in torch.fx](https://github.com/pytorch/pytorch/blob/2c4b6ec45723ebc184d958c12f56961ee2fa6ca4/torch/fx/symbolic_trace.py#L284).

**Are other areas affected or would the workaround have negative consequences in some way?**

cc @ezyang @gchanan @albanD @mruberry @walterddr @mikaylagawarecki @jbschlosser