bug fix in group conv and add test for group conv #17
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Convolution was previously working and tested only for
groups=1. When using other values forgroupsthis gave incorrect results, for example testing Clifford 1d group convolution withg = [-1]against complex group convolution gave different results.The issue was caused by the data layout.
Let's consider a 1d convolution for simplicity. Then the weight for this convolution has shape:
weight.shape =[number_of_blades, output_channels, input_channels / groups, kernel_size]The corresponding clifford kernel has shape:
kernel.shape = [number_of_blades * output_channels, number_of_blades * input_channels / groups, kernel_size]The shape of the output of the convolution is:
output.shape = [batch_size, number_of_blades * output_channels, output_length]Where it is expected that
output[:output_channels]is the first blade andoutput[output_channels:2 * output_channels]is the second blade. However this is not what is happening, since each filter hasnumber_of_blades * input_channels / groupschannels and:g[0](those are the filters expected to generate the scalar part of the output)In grouped convolution with 2 groups, the first half of the filters is assigned to the first half of the input channels, while the second half of the filters is assigned to the second half of the input channels. Each filter always expect the first half of its input channels to be the scalar part and the second part to be the e1 part, however it happens that all of its input channels are actually either all scalar or all e1!
To make group convolution work correctly, scalar and e1 channels should be interleaved to match the kernel. This reasoning can be generalised for all the other clifford algebras, and this was implemented in the last commit. Moreover a test was added to test Clifford group convolution with
g = [-1]against complex group convolution. This test now passes, while it was previously failing. All the other tests are still passing as before.