Unify keyword argument names for convolution and pooling

[f0k]

The keyword argument names for 1D and {2,3}D convolution and pooling are currently not the same.

• 1D-convolution has filter_length and stride
• {2,3}D-convolution has filter_size and strides
• {1,2,3}D-pooling has ds and st in Add support for striding and padding to pooling layers #199

I'd suggest to use filter_size and stride for all convolutions, and pool_size and stride for all poolings.
For a really uniform interface, it would be nice if all of filter_size, pool_size and stride either accepted a single integer or a tuple of integers matching the dimensionality.

Arguments for or against it?

[ebenolson]

how about copying Caffe and using kernel_size for both conv and pool?

[benanne]

Sounds like a good compromise. And I suppose having them be consistent across dimensionalities makes the interface a bit 'smaller', so I'm in favour.

Having all of them accept both tuples and single integers is also a great idea, and shouldn't be too complicated to implement cleanly.

how about copying Caffe and using kernel_size for both conv and pool?

kernel_size makes no sense for pooling operations imo, there is no kernel involved.

[benanne]

Maybe we could have a helper function that takes an integer or tuple as input, as well as the desired dimensionality. If the first input is a tuple, it checks if it has the right length and simply returns it. If the first input is an integer, it turns it into a tuple of the correct length.

This could be used in both the pooling and the convolution implementations.

[ebenolson]

yeah, I did something like that here but it would be a good helper function.

[f0k]

I like @ebenolson's approach of accepting any iterable, not just tuples. We could do that in more places. (But that's something we can always add later on, as it doesn't break backwards compatibility.) And speaking of helper functions, there's a lot more code duplication in the convolution layers that could use some refactoring, but that's also not critical for the first release.

Anyone please feel free to rename the keyword arguments as discussed above and/or add the integer/tuple handling, I'm currently spending the little Lasagne time I have on #182. It's a low-hanging fruit we can leave open for a bit until somebody comes a long and picks it. If you're a possible new contributor reading this, please step closer :)

[benanne]

I wouldn't leave this open for more than a couple of days though, ds -> pool_size is going to be a breaking change that affects a lot of code and it would be nice to have a warning for that for at least a week or so :)

[ebenolson]

I can take care of this. Do we want people's code to break now, or continue to accept the old parameter names and give a deprecation warning?

[benanne]

Thinking about it again, adding deprecation warnings for this is going to clutter the code a lot, and it would only be for a few weeks anyway. I don't think it's necessary to spend time on that. We'll post a bulletin on the mailing list when it's released.