Filter layer #1482

mtamburrano · 2014-11-26T18:44:48Z

Filter_layer developed as discussed in #1448, implemented following the suggestions by @sguada and @longjon (paragraph 2(i)).

This layer accepts 2+ bottom blobs, the first blob acts as a selector, so it should be a vector s={0,1,2, .. n} in [0,1], and the remaining blobs are blobs to be filtered. Each value of 1 in the selector vector means that the items at the corresponding indices int the bottom_blobs[1+] will be kept, on the contrary a values of 0 means the items with corresponding symbols will be filtered out.
So only non-filtered items will be forwarded and afterwards will receive backpropagation.

As param the filter_layer needs a need_back_prop vector. This is needed because some blob could contains labels, and therefore will not need backpropagation. The need_back_prop vector as the same size as the top vector, and accepts 1s and 0s depending on whether it should be backpropagated or not.

This PR needs #1483 and #1484

longjon · 2014-12-01T05:42:07Z

include/caffe/common_layers.hpp

@@ -190,6 +190,71 @@ class EltwiseLayer : public Layer<Dtype> {
 };

 /**
+ * @brief Takes two Blob%s, computes the argmax of the IF bottom blob,


Is this out-of-date?

yes it is, totally missed it, thanks

longjon · 2014-12-01T06:27:34Z

I took a first pass... it looks like the core implementation is there, but it needs some fixes as noted.

I'm confused about whether need_back_prop_ does anything post-#1483?

sguada · 2014-12-01T12:40:38Z

include/caffe/common_layers.hpp

+ protected:
+  /**
+   * @param bottom input Blob vector (length 2+)
+   *   -# @f$ (N \times C \times H \times W) @f$


The selector should be (N \times 1 \times 1 \times 1), isn't it?
I'm not sure if the selector should be the first bottom blob or the last one.

yes it should be, I'll edit this.
I don't see differences between first and last position, just say me what you prefer ;)

mtamburrano · 2014-12-01T12:50:40Z

thank you for your detailed review, I'll fix the code waiting for your further replies where needed

sguada · 2014-12-01T13:16:31Z

src/caffe/layers/filter_layer.cpp

+
+    for (size_t n = 0; n < new_tops_num; n++) {
+      int offset = indices_to_forward_[n];
+      int data_offset_top = dim*n;


Try to use predefined methods for offset
int data_offset_top = top[b-1]->offset(n);

sguada · 2014-12-01T13:22:59Z

I don't know how the test passed given the problems with lint, and the error with indices_to_forward in the backward pass.

sguada · 2014-12-01T13:39:07Z

src/caffe/test/test_filter_layer.cpp

+
+TYPED_TEST(FilterLayerTest, TestGradient) {
+  typedef typename TypeParam::Dtype Dtype;
+  if (Caffe::mode() == Caffe::CPU || sizeof(Dtype) == 4) {


It should work independently of the sizeof(Dtype) and also for GPU

I'm not sure about lint errors because make lint doesn't show anything (everything seems ok). About the indices_to_forward error, I think it access random data when n > indice_to_forward.size() and then always enter in (n != offset) case, I'll fix this.

mtamburrano · 2014-12-01T17:07:26Z

pushed various fixes discussed.
~~Now the gpu calls fail because caffe_set doesn't properly work in GPU mode, I opened #1511 to fix this.~~
The filter is still applied in Reshape instead Forward and need_back_prop vector is still a param because I think we need further discussion about that

sguada · 2014-12-03T00:52:23Z

src/caffe/layers/filter_layer.cpp

+        "Selector blob (bottom[0]) must have height == 1";
+  int num_items = bottom[0]->num();
+  for (int i = 1; i < bottom.size(); i++) {
+    CHECK_EQ(num_items, bottom[i]->num()) <<


I think CHECK_EQ(bottom[0]->num(), bottom[i]->num()) is more clear

sguada · 2014-12-03T01:25:12Z

src/caffe/layers/filter_layer.cpp

+  for (int i = 1; i < propagate_down.size(); i++) {
+    // bottom[0] is the selector and never needs backpropagation
+    // so we can start from i = 1 and index each top with i-1
+    if (propagate_down[i] && need_back_prop_[i-1]) {


It is weird that propagate_down and need_back_prop don't align.

they don't align because need_back_prop size is equals to bottom.size() - 1 (or top.size()), instead propagate_down has size equals to bottom.size().
This happens because bottom_selector never needs backprop and doesn't have a corresponding top, so is useless to force the user to specify '0' on the first element in the prototxt params

Once the selector is the last bottom they will align.

sguada · 2014-12-03T01:26:03Z

@mtamburrano Have you tried what happen if you don't use need_prop_down?

sguada · 2014-12-03T01:27:00Z

src/caffe/layers/filter_layer.cu

+      int data_offset_top = top[b-1]->offset(n);
+      int data_offset_bottom =  bottom[b]->offset(indices_to_forward_[n]);
+
+      caffe_copy(dim, bottom_data+data_offset_bottom,


add spaces around '+' and try fit in one line

mtamburrano · 2014-12-03T20:35:08Z

Have you tried what happen if you don't use need_prop_down?

Happens that blobs that not needs backprop (e.g. labels) receive it.
Until the way propagate_down is computed doesn't change (as you early proposed here and as discussed in #1483) I don't think that need_back_prop vector could be removed.

This is very complicated logic to do the backward propagation. I think it could done much easier using bottom_data_selector

I'm not sure what are you proposing... We need to fill with zeros the diff matrix on indices of filtered items and these indices are stored in indices_to_forward_, I could pick them from bottom_selector instead, but I don't think this is what you are suggesting

mtamburrano · 2014-12-05T18:35:23Z

moved selector blob to last bottom position, so now top and bottom blobs indices are aligned.

bhack · 2014-12-15T13:49:25Z

@sguada @longjon @shelhamer We need to allocate time on this in our weekly working plan. Can you give us some feedbacks of the review plan or what kind of work is still needed?

sguada · 2014-12-16T23:24:15Z

@bhack @mtamburrano the introduction of need_back_prop makes thing messier. The code for backward is also complicated, in its current form.

Could you try to use two different filter layers, one for data and another for label, which should behave properly, propagating down for data blobs but not for label blobs.

mtamburrano · 2014-12-17T16:26:21Z

@sguada, I don't think this solution works.
Both data and labels filter layers would be attached to a selector, obtained from a layer that needs back_prop, so both layers will need back_prop too.
I don't know if this is clear, let's say a branch of our net ends with an inner_product layer (remember the cat /not_cat example?) from which the selector is computed using a sigmoid followed by a threshold layer (like you suggested). Now, the inner_product is also attached to a sigmoid_cross_entropy_loss layer and needs back_prop, then all the successive layers will have the need_backward flag raised, and so the filter layers.
To avoid a back_prop, the label's filter layer should be only attached directly to data layers, but this is not possible because of the need of the selector

bhack · 2014-12-18T23:10:17Z

I saw a very very low activity by core developers (BVLC members) also after cvpr 2014 deadline.
What happens? @shelhamer Can you give some general feedback to the community (if we can consider to have ones)? We was quite responsive on every PR that we have opened and we are investing working hours on this but at this rate it very hard to reserve activity in the planning. I really hope the the "final solution" will not be that you are changing the infrastructure so that everyone can maintain its own layer with python prototypes.

bhack · 2015-02-07T17:45:02Z

@shelhamer Can you pass here?

mtamburrano · 2015-03-06T12:24:12Z

closed, new PR is #2054

mtamburrano added 3 commits November 25, 2014 17:59

implementd filter_layer

6366570

backpropagation implemented

78140d0

added tests and removed lint errors

441e292

This was referenced Nov 26, 2014

Removed propagate labels check in loss layers #1483

Closed

net.cpp now allows zero-sized batches #1484

Closed

Conditional layer #1448

Closed

fix on travis build

0340047

longjon reviewed Dec 1, 2014
View reviewed changes

longjon mentioned this pull request Dec 1, 2014

Switch layer #1496

Closed

sguada reviewed Dec 1, 2014
View reviewed changes

various fixes

f1a390b

mtamburrano added 2 commits December 1, 2014 19:04

warning fixes

78a376e

introduced caffe_gpu_set

4372bf4

sguada reviewed Dec 3, 2014
View reviewed changes

mtamburrano added 3 commits December 3, 2014 21:04

various fixes

4e2a213

removed spaces

c67f5ae

lint fix

193cc00

moved selector blob from first to last bottom blob

1cc61cd

ducha-aiki mentioned this pull request Dec 21, 2014

Deconvolution layer? #1610

Closed

shelhamer added the sandbox label Dec 30, 2014

bhack mentioned this pull request Feb 10, 2015

Next: release candidater #1849

Merged

bhack mentioned this pull request Feb 20, 2015

Allow self-contained development of Caffe layers #1896

Open

mtamburrano closed this Mar 6, 2015

This was referenced Mar 6, 2015

Removed propagate labels check in loss layers #2052

Closed

Filter layer rebased #2054

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filter layer #1482

Filter layer #1482

mtamburrano commented Nov 26, 2014

longjon Dec 1, 2014

mtamburrano Dec 1, 2014

longjon commented Dec 1, 2014

sguada Dec 1, 2014

mtamburrano Dec 1, 2014

mtamburrano commented Dec 1, 2014

sguada Dec 1, 2014

mtamburrano Dec 1, 2014

sguada commented Dec 1, 2014

sguada Dec 1, 2014

mtamburrano Dec 1, 2014

mtamburrano commented Dec 1, 2014

sguada Dec 3, 2014

sguada Dec 3, 2014

mtamburrano Dec 3, 2014

sguada Dec 4, 2014

sguada commented Dec 3, 2014

sguada Dec 3, 2014

mtamburrano commented Dec 3, 2014

mtamburrano commented Dec 5, 2014

bhack commented Dec 15, 2014

sguada commented Dec 16, 2014

mtamburrano commented Dec 17, 2014

bhack commented Dec 18, 2014

bhack commented Feb 7, 2015

mtamburrano commented Mar 6, 2015

Filter layer #1482

Filter layer #1482

Conversation

mtamburrano commented Nov 26, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

longjon commented Dec 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtamburrano commented Dec 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sguada commented Dec 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtamburrano commented Dec 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sguada commented Dec 3, 2014

Choose a reason for hiding this comment

mtamburrano commented Dec 3, 2014

mtamburrano commented Dec 5, 2014

bhack commented Dec 15, 2014

sguada commented Dec 16, 2014

mtamburrano commented Dec 17, 2014

bhack commented Dec 18, 2014

bhack commented Feb 7, 2015

mtamburrano commented Mar 6, 2015