优化LayerList类的insert函数

[Qin-sx]

PR Category

User Experience

PR Types

Improvements

Description

优化LayerList类的insert函数
对应issue

insert函数在对index判断时没有考虑_sub_layers为空的情，即[0,0)。

assert isinstance(index, int) and -len(self._sub_layers) <= index < len(
            self._sub_layers
        ), f"index should be an integer in range [{-len(self)}, {len(self)})"

在pytorch中，并没有进行类似的判断，而是直接插入新的layer。

    def insert(self, index: int, module: Module) -> None:
        r"""Insert a given module before a given index in the list.

        Args:
            index (int): index to insert.
            module (nn.Module): module to insert
        """
        for i in range(len(self._modules), index, -1):
            self._modules[str(i)] = self._modules[str(i - 1)]
        self._modules[str(index)] = module

本次优化将insert函数的插入范围改为了左闭右闭区间，例如[0,1],[0,0]等。修改后：

1. 在_sub_layers为空时，可以index=0插入新的layer。
2. 可以在位置为len(self)处的插入新的layer。

加入了相应的测试。

[SigureMo]

这和 SOT 有关么？为什么要加在这里？

[Qin-sx]

您好，因为我在test文件夹搜了一下，貌似只有SOT中有LayerList类的相关测试。

[SigureMo]

那就去 test/legacy_test 自己新增一个单测，从单测名就可以明显看出在这里加是不合适的

[Qin-sx]

收到，已修改

[zhwesky2010]

下面的assert可以挪到上面来，做一个整体的assert判断。另外这里能否不加额外分支，和下面代码采用相同实现？

[Qin-sx]

收到，已修改，_get_abs_idx函数也没有考虑index=0的情况，也进行了修改。

[zhwesky2010]

这个是不是因为原来的阈值范围不合理，原来是：[-len, len) 前开后闭，应改成 [-len, len]，对于位置为len处的插入就是在最末尾插入。你看下torch是不是这样设计的

也就是新增一个功能：在位置为len处的插入，不仅仅是针对len为0，而是一个通用功能

[Qin-sx]

pytorch

pytorch中貌似并没有考虑这个问题。
我在pytorch的insert函数中加入了打印：

    def insert(self, index: int, module: Module) -> None:
        r"""Insert a given module before a given index in the list.

        Args:
            index (int): index to insert.
            module (nn.Module): module to insert
        """
        print("inner Pytorch ModuleList insert function: ",self._modules)
        for i in range(len(self._modules), index, -1):
            self._modules[str(i)] = self._modules[str(i - 1)]
            
        self._modules[str(index)] = module

并对以下代码进行了测试：

import torch
modules = torch.nn.ModuleList()
modules.insert(2, torch.nn.Linear(20, 20))
print("outer modules:", modules)
modules.insert(3, torch.nn.Linear(30, 30))
print("outer modules:", modules)
modules.insert(0, torch.nn.Linear(10, 10))
print("outer modules:", modules)

测试代码出现了报错，应该是因为没有对输入的str(i)进行限制。

inner Pytorch ModuleList insert function:  OrderedDict()
outer modules: ModuleList(
  (0): Linear(in_features=20, out_features=20, bias=True)
)
inner Pytorch ModuleList insert function:  OrderedDict([('2', Linear(in_features=20, out_features=20, bias=True))])
outer modules: ModuleList(
  (0): Linear(in_features=20, out_features=20, bias=True)
  (1): Linear(in_features=30, out_features=30, bias=True)
)
inner Pytorch ModuleList insert function:  OrderedDict([('2', Linear(in_features=20, out_features=20, bias=True)), ('3', Linear(in_features=30, out_features=30, bias=True))])
Traceback (most recent call last):
  File "/home/qinsx/paddle_test/Modulelist/modulelist.py", line 11, in <module>
    modules.insert(0, torch.nn.Linear(10, 10))
  File "/home/qinsx/pyenv/pdbaddbmm/lib/python3.8/site-packages/torch/nn/modules/container.py", line 377, in insert
    self._modules[str(i)] = self._modules[str(i - 1)]
KeyError: '1'

paddle

在paddle中，insert函数调用了_get_abs_idx函数来获取id。

    def _get_abs_idx(self, idx: int) -> int:
        if isinstance(idx, int):
            if not (len(self) == 0 and idx == 0) and not (
                -len(self) <= idx < len(self)
            ):
                raise IndexError(
                    f'index {idx} is out of range, should be 0 for empty list or in range [{-len(self)}, {len(self)})'
                )
            if idx < 0:
                idx += len(self)
        return idx

_get_abs_idx函数也会对左闭右开进行检查。我个人认为_get_abs_idx函数应该保持左闭右开，因为如果其他函数在调用时，如果获取右区间最大值会越界。我加入的特殊情况为length为0时返回idx=0。
我认为现在可以有两种方案：

1. 方案一：保持这种修改，即对_get_abs_idx和insert函数加入length为0的特殊情况判断。
2. 方案二：不修改_get_abs_idx函数。在insert函数中不调用_get_abs_idx函数，而仿照_get_abs_idx函数单独进行判断。

方案二代码

        assert isinstance(index, int) and -len(self._sub_layers) <= index <= len(
            self._sub_layers
        ), f"index should be an integer in range [{-len(self)}, {len(self)}]"

        if idx < 0:
            idx += len(self)
        for i in range(len(self._sub_layers), index, -1):
            self._sub_layers[str(i)] = self._sub_layers[str(i - 1)]
        self._sub_layers[str(index)] = sublayer

[zhwesky2010]

@Qin-sx 我可能没描述清楚，新增的功能是指：在位置为len(self)处的插入，比如长度为0的，在0位置插入，长度为1的，在1位置插入。

试了下torch，这个功能是支持的。

[zhwesky2010]

@Qin-sx 按方案二实现吧，如果_get_abs_idx不能修改为前闭后闭，那就不用调这个函数。

[Qin-sx]

收到，已修改，并对新增的在位置为len(self)处的插入功能加入了测试。

[zhwesky2010]

LGTM