Skip to content
This repository has been archived by the owner on Oct 30, 2019. It is now read-only.

Minor bugfix for type A shortcut #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lim0606
Copy link

@lim0606 lim0606 commented Apr 27, 2016

The original one gave me an error when shorcut = type A and nOutputPlane ~= nInpuPlane x 2 (when opt.dataset = imagenet).

This might not be the optimal solution, but I think it could be a palliative for now.

Best regards,

Jaehyun

The original one gave me an error when shorcut = type A and nOutputPlane ~= nInpuPlane x 2 (when opt.dataset = imagenet). 

This might not be the optimal solution, but I think it could be a palliative for now. 

Best regards, 

Jaehyun
@ghost
Copy link

ghost commented Apr 27, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@ghost
Copy link

ghost commented Apr 27, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ghost ghost added the CLA Signed label Apr 27, 2016
@colesbury
Copy link
Contributor

Thanks for the PR and the bug description. This looks like it works, but I'm not thrilled with the idea of using the convolution to add the zero-valued feature planes. We should probably just add a sub-class nn.Module to do the zero-padding.

@lim0606
Copy link
Author

lim0606 commented Apr 28, 2016

@colesbury
Exactly! However, I couldn't think of a better idea for broaching this bug rather than adding (temporaray) solution :)

Best regards,

Jaehyun

@ghost ghost added the CLA Signed label Jul 12, 2016
@xichangzun
Copy link

it did work in my code, thanks a lot

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants