Added TemporalRowConvolutionMM layer, tests, and documentation #1100
Conversation
Resolved merge conflicts
|
This is ready for you to review. |
There was a problem hiding this comment.
just FYI, this is a 1d version of SpatialConvolutionMap with a one-to-one connection table (commonly used in many papers including LeCun'98 i think) :)
Please rename the layer to TemporalRowConvolution, drop the MM suffix. We had spent quite some time to get rid of the MM suffix and it's history across the nn repo, do not want this to become another historical naming burden.
| } | ||
| } | ||
|
|
||
| static int THNN_(view_weight_rowconv)(THTensor **_weight) { |
There was a problem hiding this comment.
why is this function really needed?
You are defining this module afresh, you can control the exact shape of the weight. Just define the weight shape as you think is appropriate and let's stick to it.
There was a problem hiding this comment.
Removed the function and fixed the weight to a static shape in the Lua interface.
| ++dimF; | ||
| } | ||
|
|
||
| THTensor *tinput = THTensor_(newTranspose)(input, dimS, dimF); |
There was a problem hiding this comment.
the way you wrote this function with all the transposes, it'd be a shame to not also have the function support feature-dimension be 2nd dim optionally (the way it's in pytorch, i.e. Batch x Channels x Width), because when you want to use this in to pytorch, Conv1d's output can be sent directly into this.
Consider adding a boolean flag to the function signature that controls this transposition
There was a problem hiding this comment.
It now supports nBatchFrame x inputFrameSize x nInputFrame by supplying an optional fourth argument featFirst = true or by setting module.featFirst = true.
| else | ||
| stdv = 1 / math.sqrt(self.kW * self.inputFrameSize) | ||
| end | ||
| if nn.oldseed then |
There was a problem hiding this comment.
you can remove the oldseed stuff
|
thanks! :) |
Added row convolutions (a.k.a. lookahead convolutions) as described in this paper.