yaoppeng / U-Net_v2

214 stars 20 forks source link

Using the same Conv2d layer instance may cause problems #10

Closed Rinsonlaw closed 11 months ago

Rinsonlaw commented 11 months ago

I notice that there is a subtle problem in the initialization part of SDI layer.

Using the code self.convs = nn.ModuleList([nn.Conv2d(channel, channel, kernel_size=3, stride=1, padding=1)] * 4) in Python results in creating a list with four references to the same nn.Conv2d instance because of the list multiplication ([conv] * 4).

This means that instead of having four independent nn.Conv2d instances, all four entries in the module list actually point to the same object and share the same set of weights and biases.

class SDI(nn.Module):
    def __init__(self, channel):
        super().__init__()

        self.convs = nn.ModuleList(
            [nn.Conv2d(channel, channel, kernel_size=3, stride=1, padding=1)] * 4)

And the same problem can be also found in self.seg_outs = nn.ModuleList([nn.Conv2d(channel, n_classes, 1, 1)] * 4).

The correct approach is to use a list comprehension to create separate nn.Conv2d instances:

self.convs = nn.ModuleList(
    [nn.Conv2d(channel, channel, kernel_size=3, stride=1, padding=1) for _ in range(4)]
)
yaoppeng commented 11 months ago

Thank you for the valuable recommendation.

I corrected it. Seems like the results are not impacted too much (slightly better than before). I will check it further.