Conv2D and groups > 1 about lora

[edwardjhu]

Thanks everyone! This is indeed a bug.

Because the LoRA module in this case doesn't have a convolution structure, I don't see a reason to prefer one fix over another.

It's fixed now. 375704a

[ExponentialML]

Since it seems to be related, there's a possible issue when it comes to scaling Conv2d.
It seems that when creating new parameters, we have:

self.lora_A = nn.Parameter(
                self.weight.new_zeros((r*kernel_size, in_channels*kernel_size))
            )
self.lora_B = nn.Parameter(
    self.weight.new_zeros((out_channels*kernel_size, r*kernel_size))

The kernel size here is multiplied by the rank. We then scale this by self.scale = alpha / r.
I believe the appropriate solution should be something like:

scaled_rank = r * kernel_size
self.scale = alpha *  (r // scaled_rank)

Update:

This is no longer a problem for me, but could still be a viable option depending on the use case.

[yousuf907]

I think the input_channels need to be divided by groups without modifying output channels since input channels are modified in nn.Conv2d weights of shape: [output_channels, input_channels / groups, kernel_size[0], kernel_size[1]].
Hence,
self.lora_A = nn.Parameter(self.weight.new_zeros((r*kernel_size, in_channels//self.groups*kernel_size))
self.lora_B = nn.Parameter(self.weight.new_zeros((out_channels*kernel_size, r*kernel_size))

[bbbxyz]

Thanks @edwardjhu !