Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix for ConcatLayer with propagate_down set #2972

Merged
merged 2 commits into from
Aug 25, 2015

Conversation

jeffdonahue
Copy link
Contributor

Currently, if propagate_down[i] is false, offset_concat_axis is not correctly
updated for any subsequent bottoms (i+1, i+2, ...).

This bug is quite serious for anyone whom it affects -- if you use concat layer and some of its inputs don't need backpropagation (i.e. because they have no learned parameters below them), you should pull and try again with this fix. Many thanks to @bharath272 for reporting.

This was my bug -- it's been broken since merging #1970 in which I (tried to) generalize blobs and many layers to N dimensions. Sorry for the trouble. In the near future I plan to go through all multi-bottom layers and at least glance to make sure they don't have similar propagate_down related bugs, since we don't ever really test the propagate_down functionality. Edit: After going through the list, I don't think any other (official) Caffe layers are subject to this flavor of bug, but please let me know if I missed anything.

@shelhamer shelhamer added the bug label Aug 25, 2015
if propagate_down[i] was set, offset_concat_axis was not correctly
updated for subsequent bottoms i+1, i+2, ...
@jeffdonahue
Copy link
Contributor Author

The test added in the first commit should cause the build to fail, and even did so in the build on my public fork, but somehow passes here... Not sure what's up with that. Regardless, we should probably merge this ASAP.

@shelhamer
Copy link
Member

Thanks for the fix @jeffdonahue and reporting the issue @bharath272. To echo Jeff, nets with a concat layer that does not backprop to all inputs should be re-examined!

shelhamer added a commit that referenced this pull request Aug 25, 2015
[fix] properly backprop through ConcatLayer with propagate_down set
@shelhamer shelhamer merged commit c54f2c4 into BVLC:master Aug 25, 2015
@jeffdonahue jeffdonahue deleted the concat-backward-fix branch August 26, 2015 01:05
myfavouritekk added a commit to myfavouritekk/caffe that referenced this pull request Aug 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants