Moving to theano.tensor.nnet.abstract_conv2d.conv2d

[jonhoo]

Theano/Theano#3618 was recently merged, bringing some substantial performance improvements to conv2d. However, the new implementation is only used if heano.tensor.nnet.abstract_conv2d.conv2d is used. According to this mailing list post, the default convolution may also to change to abstract_conv2d soon. Any chance of Lasagne making the change?

[benanne]

I am wary of making the switch before Theano itself switches this to be the default, to be honest. Is there any info about the performance improvements anywhere? The PR you linked just adds the optimizations to swap in the corrMM implementation, but I don't see anything about performance there.

We should probably set up a game plan for this, as it's a bit more complicated than just updating the Conv2DLayer implementation. Ideally, we would be able to get rid of Conv2DDNNLayer and Conv2DMMLayer altogether, with Theano deciding which underlying implementation to use, as it should be. We will have to determine whether this is indeed the case. Of course we would still have to provide aliases (with deprecation warnings) so we don't break people's code.

Conv2DCCLayer will probably have to remain, seeing as this implementation is not part of Theano itself.

We'll also have to transition the 1D layers, and maybe this would be a good time to add a proper 3D convolutional layer as well.

[jonhoo]

Ah, I got the performance from this other issue. While it's true that they haven't made it the default yet, the fact that it's been merged should at least inspire some confidence, no?

[benanne]

I see! I guess OpenMP only affects CPU performance, but still, this would probably be useful to have. We can at least start planning the switch.

[f0k]

While it's true that they haven't made it the default yet, the fact that it's been merged should at least inspire some confidence, no?

But why should we use T.nnet.abstract_conv2d.conv2d in Lasagne if T.nnet.conv2d will be replaced by it anyway? I think this is something we should just wait for to happen upstream in Theano.

We'll also have to transition the 1D layers, and maybe this would be a good time to add a proper 3D convolutional layer as well.

Theano doesn't have a proper conv3d equivalent yet, though. There are several implementations, but no common interface and automatic replacements. The best bet for now is to add a Conv3DDNNLayer and Pool3DDNNLayer.

[benanne]

Yeah, I guess you're right. It would be nice to have a single unified interface for convolutions. At least abstract_conv2d is a step in the right direction :)

[f0k]

Theano switched to using the placeholder convolutions by default now (Theano/Theano#3721 and Theano/Theano#3729), so we can close this.

[benanne]

Excellent! Maybe we should set up a new issue though, to discuss how we plan to revamp our current convolutional layer implementations in response to this. I imagine we can get rid of some of the padding logic, and maybe we can even eliminate Conv2DDNNLayer and Conv2DMMLayer.

[jonhoo]

@benanne: ah, that would be great. Having this at the top of all my networks is cumbersome:

Conv2DLayer = lasagne.layers.Conv2DLayer
MaxPool2DLayer = lasagne.layers.MaxPool2DLayer
if theano.config.device.startswith("gpu"):
    import lasagne.layers.dnn
    # Force GPU implementations if a GPU is available.
    if theano.sandbox.cuda.dnn.dnn_available():
        Conv2DLayer = lasagne.layers.dnn.Conv2DDNNLayer
        MaxPool2DLayer = lasagne.layers.dnn.MaxPool2DDNNLayer

[f0k]

Maybe we should set up a new issue though, [...]

Yes, I thought so as well, but didn't have the time. Now: #524

@jonhoo: You can get rid of this logic right now when you update to the latest Theano master. Using a Conv2DLayer shouldn't be any slower than using a Conv2DDNNLayer then, even with subsampling and/or padding. Note that Conv2DDNNLayer does a correlation by default, while Conv2DLayer does a convolution -- so be careful when reusing previously-learned weights.