ndif-team/nnsight

[Bug] RecursionError in NNsightModel initialization

Closed this issue · 1 comments

NNsightModel object initialization is broken.

Code Example:

from collections import OrderedDict
from nnsight import NNsight
import torch

input_size = 5
hidden_dims = 10
output_size = 2

torch.manual_seed(423)

net = torch.nn.Sequential(
    OrderedDict(
        [
            ("layer1", torch.nn.Linear(input_size, hidden_dims)),
            ("layer2", torch.nn.Linear(hidden_dims, output_size))
        ]
    )
).requires_grad_(False)

input = torch.rand((1, input_size))

model = NNsight(net)

Error:

Traceback (most recent call last):
  File “<project-path>/test.py", line 22, in <module>
    model = NNsight(net)
  File “<env-path>/lib/python3.10/site-packages/nnsight/models/NNsightModel.py", line 51, in __init__
    self._model_key = model_key
  File “<env-path>/lib/python3.10/site-packages/nnsight/models/NNsightModel.py", line 307, in __setattr__
    setattr(self._envoy, key, value)
  File “<env-path>/lib/python3.10/site-packages/nnsight/models/NNsightModel.py", line 319, in __getattr__
    return getattr(self._envoy, key)
  File “<env-path>/lib/python3.10/site-packages/nnsight/models/NNsightModel.py", line 319, in __getattr__
    return getattr(self._envoy, key)
  File “<env-path>/lib/python3.10/site-packages/nnsight/models/NNsightModel.py", line 319, in __getattr__
    return getattr(self._envoy, key)
  [Previous line repeated 993 more times]
RecursionError: maximum recursion depth exceeded.

The issue seems related to this commit ae30799 . The added changes are meant to add sub-envoys using the overloaded setattr functions, but, it is being called before a parent Envoy has been set within the NNsightModel class, as seen here: https://github.com/ndif-team/nnsight/blob/main/src/nnsight/models/NNsightModel.py#L78.


Potential fix:

This section of the code (https://github.com/ndif-team/nnsight/blob/main/src/nnsight/models/NNsightModel.py#L305-L307) seems unnecessary because the logic is already implemented in the Envoy class to use setattr for adding new sub-envoys. Commenting it makes the example run without producing an error, however, I am not sure if this interferes with the intended improvement of the commit related to this issue.

@AdamBelfki3 Good catch thank you! Just pushed a fix. We definitely need a base NNsight model test added to the pytests