Add assignment support for Sequential #4931
Conversation
torch/nn/modules/container.py
Outdated
| return next(it) | ||
|
|
||
| def __setitem__(self, idx, module): | ||
| return setattr(self, str(idx), module) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Simply manipulate the variable returned by iterator only refers to a new module object, rather than modify the existing one.
torch/nn/modules/container.py
Outdated
| # Specify the module by index | ||
| model[1] = nn.Sigmoid() | ||
| # Specify the module by name if initialized with OrderedDict | ||
| model.relu1 = nn.LeakyReLU() |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
torch/nn/modules/container.py
Outdated
| def _get_item_by_idx(self, iterator, idx): | ||
| """Get the idx-th item of the iterator""" | ||
| size = len(self) | ||
| if not (-size <= idx < size): |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
torch/nn/modules/container.py
Outdated
| for i in range(idx): | ||
| next(it) | ||
| return next(it) | ||
| return self._get_item_by_idx(self._modules.items(), idx) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
apaszke
left a comment
apaszke
left a comment
There was a problem hiding this comment.
Looks good now, but I forgot to ask for tests for this feature. Can you please add them?
|
How do you think of using pure items = list(args)
for item in items:
if isinstance(item, tuple):
...
else:
... |
|
@pytorchbot test this please |
|
No, let's not complicate the API. This looks good as is. Thanks! |
3 participants
Simply a copy from
ModuleList.I think the
Sequentialis just a forwardable version ofModuleList, it makes sense to combine both into one class.