Lasagne / Recipes

Lasagne recipes: examples, IPython notebooks, ...
MIT License
914 stars 418 forks source link

Wrong order of stride and pad arguments in build_simple_block #91

Closed krikru closed 7 years ago

krikru commented 7 years ago

In the function build_simple_block in resnet50.py, the stride and pad arguments to the Conv2DLayer (ConvLayer) constructor are given in the order pad then stride (if use_bias is True, not if use_bias is False), but the Conv2DLayer constructor takes those two arguments in the order stride then pad.

f0k commented 7 years ago

Good catch! So when called with use_bias=True, stride and pad are swapped: https://github.com/Lasagne/Recipes/blob/1fcc5c0/modelzoo/resnet50.py#L62-L65 As far as I see, this only happens for the very first convolutional block: https://github.com/Lasagne/Recipes/blob/1fcc5c0/modelzoo/resnet50.py#L155-L157 This has 64 filters of size 7 with stride 3 and pad 2, which is then swapped to stride 2 and pad 3. The latter is consistent with Kaiming's code: https://github.com/KaimingHe/deep-residual-networks/blob/master/prototxt/ResNet-50-deploy.prototxt#L14-L17 So the model is correct, but the code is of course not okay.

Would you like to file a PR fixing the order in the two code snippets I linked to? If you're not comfortable with git, you can use the pencil icon at the top right of https://github.com/Lasagne/Recipes/blob/master/modelzoo/resnet50.py to edit the file in your browser. Let me know if we should take care of this ourselves instead. Thank you!

krikru commented 7 years ago

I haven't used the model myself, I have just looked at the code, and I'm probably not going to file a PR unless I actually use the code myself, which I'm not sure I'm going to do. So sorry, but if you want this fixed it's probably best that someone else does it :)

f0k commented 7 years ago

Fair enough -- thank you for taking the time to report it! I've filed a PR myself now.