Crop: fixes, tests and negative axis indexing.

diff --git a/include/caffe/layers/crop_layer.hpp b/include/caffe/layers/crop_layer.hpp
index f84bab1..5c605b2 100644 (file)
--- a/include/caffe/layers/crop_layer.hpp
+++ b/include/caffe/layers/crop_layer.hpp
@@ -11,8 +11,8 @@
 namespace caffe {
 
 /**
- * @brief Takes a Blob and crop it along either the width or height dimension,
- *        outputting a cropped Blob.
+ * @brief Takes a Blob and crop it, to the shape specified by the second input
+ *  Blob, across all dimensions after the specified axis.
  *
  * TODO(dox): thorough documentation for Forward, Backward, and proto params.
  */

diff --git a/src/caffe/layers/crop_layer.cpp b/src/caffe/layers/crop_layer.cpp
index 82729f1..e81bdd7 100644 (file)
--- a/src/caffe/layers/crop_layer.cpp
+++ b/src/caffe/layers/crop_layer.cpp
@@ -15,44 +15,52 @@ namespace caffe {
 template <typename Dtype>
 void CropLayer<Dtype>::LayerSetUp(const vector<Blob<Dtype>*>& bottom,
     const vector<Blob<Dtype>*>& top) {
+  // All logic that depends only on the number of dimensions is here,
+  // the rest is in Reshape because it depends on Blob size.
+  // bottom[0] supplies the data
+  // bottom[1] supplies the size
+  const CropParameter& param = this->layer_param_.crop_param();
   CHECK_EQ(bottom.size(), 2) << "Wrong number of bottom blobs.";
-  // parameter setup moved to Reshape because it depends on size.
+  int input_dim = bottom[0]->num_axes();
+  const int start_axis = bottom[0]->CanonicalAxisIndex(param.axis());
+  CHECK_LT(start_axis, input_dim) << "crop axis bigger than input dim";
+  if (param.offset_size() > 1) {
+    // the number of crop values specified must be equal to the number
+    // of dimensions following axis
+    CHECK_EQ(start_axis + param.offset_size(), input_dim)
+      << "number of offset values specified must be equal to the number of "
+      << "dimensions following axis.";
+  }
 }
 
 template <typename Dtype>
 void CropLayer<Dtype>::Reshape(const vector<Blob<Dtype>*>& bottom,
     const vector<Blob<Dtype>*>& top) {
   const CropParameter& param = this->layer_param_.crop_param();
-  // bottom[0] supplies the data
-  // bottom[1] supplies the size
   int input_dim = bottom[0]->num_axes();
-  CHECK_LT(param.axis(), input_dim) << "crop axis bigger than input dim";
+  const int start_axis = bottom[0]->CanonicalAxisIndex(param.axis());
+
   // initialize all offsets to 0
   offsets = vector<int>(input_dim, 0);
   // initialize new shape to bottom[0]
   vector<int> new_shape(bottom[0]->shape());
 
-  if (param.offset_size() > 1) {
-    // the number of crop values specified must be equal to the number
-    // of dimensions following axis
-    CHECK_EQ(param.axis() + param.offset_size(), input_dim)
-      << "number of crop values specified must be equal to the number of "
-      << "dimensions following axis.";
-  }
   // apply crops
   for (int i = 0; i < input_dim; ++i) {
     int crop_offset = 0;
     int new_size    = bottom[0]->shape(i);
-    if (i >= param.axis() && param.offset_size() == 1) {
-      // if only one crop value is supplied, crop all dimensions after axis
-      // by this crop value
-      crop_offset = param.offset(0);
-      new_size = bottom[1]->shape(i);
-    } else if (i >= param.axis() && param.offset_size() > 1) {
-      // crop values specified must be equal to the number of dimensions
-      // following axis
-      crop_offset = param.offset(i - param.axis());
+    if (i >= start_axis) {
       new_size = bottom[1]->shape(i);
+
+      if (param.offset_size() == 1) {
+        // if only one crop value is supplied, crop all dimensions after axis
+        // by this crop value
+        crop_offset = param.offset(0);
+      } else if (param.offset_size() > 1) {
+        // crop values specified must be equal to the number of dimensions
+        // following axis
+        crop_offset = param.offset(i - start_axis);
+      }
     }
     // Check that the image we are cropping minus the margin is bigger
     // than the destination image.
@@ -77,7 +85,7 @@ void CropLayer<Dtype>::crop_copy(const vector<Blob<Dtype>*>& bottom,
              Dtype* dest_data,
              bool is_forward) {
   if (cur_dim + 1 < top[0]->num_axes()) {
-    // We are not yet at the final dimension, call copy recursivley
+    // We are not yet at the final dimension, call copy recursively
     for (int i = 0; i < top[0]->shape(cur_dim); ++i) {
       indices[cur_dim] = i;
       crop_copy(bottom, top, offsets, indices, cur_dim+1,

diff --git a/src/caffe/layers/crop_layer.cu b/src/caffe/layers/crop_layer.cu
index 7b832c0..9ed8f7c 100644 (file)
--- a/src/caffe/layers/crop_layer.cu
+++ b/src/caffe/layers/crop_layer.cu
@@ -100,9 +100,6 @@ template <typename Dtype>
 void CropLayer<Dtype>::Forward_gpu(const vector<Blob<Dtype>*>& bottom,
     const vector<Blob<Dtype>*>& top) {
   std::vector<int> indices(top[0]->num_axes(), 0);
-  // This works because crop_copy uses caffe_copy which calls cudaMemcpy.
-  // My intuition is that calling this thousands of times is probably less
-  // efficient than writing a custom kernel.
   const Dtype* bottom_data = bottom[0]->gpu_data();
   Dtype* top_data = top[0]->mutable_gpu_data();
   crop_copy_gpu(bottom, top, offsets, indices, 0, bottom_data, top_data, true);

diff --git a/src/caffe/proto/caffe.proto b/src/caffe/proto/caffe.proto
index 60d387a..6900bb7 100644 (file)
--- a/src/caffe/proto/caffe.proto
+++ b/src/caffe/proto/caffe.proto
@@ -610,8 +610,10 @@ message CropParameter {
   // If only one `offset` is set, then all dimensions are offset by this amount.
   // Otherwise, the number of offsets must equal the number of cropped axes to
   // shift the crop in each dimension accordingly.
-  // Note: standard dimensions are N,C,H,W so the default is a spatial crop.
-  optional uint32 axis = 1 [default = 2];
+  // Note: standard dimensions are N,C,H,W so the default is a spatial crop,
+  // and `axis` may be negative to index from the end (e.g., -1 for the last
+  // axis).
+  optional int32 axis = 1 [default = 2];
   repeated uint32 offset = 2;
 }
 

diff --git a/src/caffe/test/test_crop_layer.cpp b/src/caffe/test/test_crop_layer.cpp
new file mode 100644 (file)
index 0000000..ba962e4
--- /dev/null
+++ b/src/caffe/test/test_crop_layer.cpp
@@ -0,0 +1,228 @@
+#include <vector>
+
+#include "gtest/gtest.h"
+
+#include "caffe/blob.hpp"
+#include "caffe/common.hpp"
+#include "caffe/filler.hpp"
+#include "caffe/layers/crop_layer.hpp"
+
+#include "caffe/test/test_caffe_main.hpp"
+#include "caffe/test/test_gradient_check_util.hpp"
+
+namespace caffe {
+
+template <typename TypeParam>
+class CropLayerTest : public MultiDeviceTest<TypeParam> {
+  typedef typename TypeParam::Dtype Dtype;
+
+ protected:
+  CropLayerTest()
+      : blob_bottom_0_(new Blob<Dtype>(2, 5, 6, 5)),
+        blob_bottom_1_(new Blob<Dtype>(2, 4, 5, 3)),
+        blob_top_(new Blob<Dtype>()) {}
+  virtual void SetUp() {
+    // fill the values
+    for (int i = 0; i < this->blob_bottom_0_->count(); ++i) {
+      this->blob_bottom_0_->mutable_cpu_data()[i] = i;
+    }
+
+
+    blob_bottom_vec_0_.push_back(blob_bottom_0_);
+    blob_bottom_vec_0_.push_back(blob_bottom_1_);
+    blob_top_vec_.push_back(blob_top_);
+  }
+
+  virtual ~CropLayerTest() {
+    delete blob_bottom_0_; delete blob_bottom_1_;
+    delete blob_top_;
+  }
+
+  Blob<Dtype>* const blob_bottom_0_;
+  Blob<Dtype>* const blob_bottom_1_;
+  Blob<Dtype>* const blob_top_;
+  vector<Blob<Dtype>*> blob_bottom_vec_0_;
+  vector<Blob<Dtype>*> blob_top_vec_;
+};
+
+
+TYPED_TEST_CASE(CropLayerTest, TestDtypesAndDevices);
+
+TYPED_TEST(CropLayerTest, TestSetupShapeAll) {
+  typedef typename TypeParam::Dtype Dtype;
+  LayerParameter layer_param;
+
+  // Crop all dimensions
+  layer_param.mutable_crop_param()->set_axis(0);
+  CropLayer<Dtype> layer(layer_param);
+  layer.SetUp(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  for (int i = 0; i < this->blob_top_->num_axes(); ++i) {
+    EXPECT_EQ(this->blob_bottom_1_->shape(i), this->blob_top_->shape(i));
+  }
+}
+
+TYPED_TEST(CropLayerTest, TestSetupShapeDefault) {
+  typedef typename TypeParam::Dtype Dtype;
+  LayerParameter layer_param;
+  // Crop last two dimensions, axis is 2 by default
+  CropLayer<Dtype> layer(layer_param);
+  layer.SetUp(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  for (int i = 0; i < this->blob_top_->num_axes(); ++i) {
+    if (i < 2) {
+      EXPECT_EQ(this->blob_bottom_0_->shape(i), this->blob_top_->shape(i));
+    } else {
+      EXPECT_EQ(this->blob_bottom_1_->shape(i), this->blob_top_->shape(i));
+    }
+  }
+}
+
+TYPED_TEST(CropLayerTest, TestSetupShapeNegativeIndexing) {
+  typedef typename TypeParam::Dtype Dtype;
+  LayerParameter layer_param;
+  // Crop last dimension by negative indexing
+  layer_param.mutable_crop_param()->set_axis(-1);
+  CropLayer<Dtype> layer(layer_param);
+  layer.SetUp(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  for (int i = 0; i < this->blob_top_->num_axes(); ++i) {
+    if (i < 3) {
+      EXPECT_EQ(this->blob_bottom_0_->shape(i), this->blob_top_->shape(i));
+    } else {
+      EXPECT_EQ(this->blob_bottom_1_->shape(i), this->blob_top_->shape(i));
+    }
+  }
+}
+
+
+TYPED_TEST(CropLayerTest, TestForwardNum) {
+  typedef typename TypeParam::Dtype Dtype;
+  LayerParameter layer_param;
+  layer_param.mutable_crop_param()->set_axis(0);
+
+  CropLayer<Dtype> layer(layer_param);
+  layer.SetUp(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  layer.Forward(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  for (int n = 0; n < this->blob_bottom_0_->num(); ++n) {
+    for (int c = 0; c < this->blob_bottom_0_->channels(); ++c) {
+      for (int h = 0; h < this->blob_bottom_0_->height(); ++h) {
+        for (int w = 0; w < this->blob_bottom_0_->width(); ++w) {
+          if ( n < this->blob_top_->shape(0) &&
+              c < this->blob_top_->shape(1) &&
+              h < this->blob_top_->shape(2) &&
+              w < this->blob_top_->shape(3) ) {
+            EXPECT_EQ(this->blob_top_->data_at(n, c, h, w),
+                this->blob_bottom_0_->data_at(n, c, h, w));
+          }
+        }
+      }
+    }
+  }
+}
+
+TYPED_TEST(CropLayerTest, TestForwardNumOffsets) {
+  typedef typename TypeParam::Dtype Dtype;
+  LayerParameter layer_param;
+  layer_param.mutable_crop_param()->set_axis(0);
+  layer_param.mutable_crop_param()->add_offset(0);
+  layer_param.mutable_crop_param()->add_offset(1);
+  layer_param.mutable_crop_param()->add_offset(1);
+  layer_param.mutable_crop_param()->add_offset(2);
+  CropLayer<Dtype> layer(layer_param);
+  layer.SetUp(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  layer.Forward(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  for (int n = 0; n < this->blob_bottom_0_->num(); ++n) {
+    for (int c = 0; c < this->blob_bottom_0_->channels(); ++c) {
+      for (int h = 0; h < this->blob_bottom_0_->height(); ++h) {
+        for (int w = 0; w < this->blob_bottom_0_->width(); ++w) {
+          if ( n < this->blob_top_->shape(0) &&
+              c < this->blob_top_->shape(1) &&
+              h < this->blob_top_->shape(2) &&
+              w < this->blob_top_->shape(3) ) {
+            EXPECT_EQ(this->blob_top_->data_at(n, c, h, w),
+                this->blob_bottom_0_->data_at(n+0, c+1, h+1, w+2));
+          }
+        }
+      }
+    }
+  }
+}
+
+TYPED_TEST(CropLayerTest, TestGradientNum) {
+  typedef typename TypeParam::Dtype Dtype;
+  LayerParameter layer_param;
+  CropLayer<Dtype> layer(layer_param);
+
+  layer.SetUp(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  layer.Forward(this->blob_bottom_vec_0_, this->blob_top_vec_);
+
+  // Copy top data into diff
+  caffe_copy(this->blob_top_->count(), this->blob_top_->cpu_data(),
+             this->blob_top_->mutable_cpu_diff());
+
+  // Do backward pass
+  vector<bool> propagate_down(2, true);
+  layer.Backward(this->blob_top_vec_, propagate_down, this->blob_bottom_vec_0_);
+
+
+  // Check results
+  for (int n = 0; n < this->blob_bottom_0_->num(); ++n) {
+    for (int c = 0; c < this->blob_bottom_0_->channels(); ++c) {
+      for (int h = 0; h < this->blob_bottom_0_->height(); ++h) {
+        for (int w = 0; w < this->blob_bottom_0_->width(); ++w) {
+          if ( n < this->blob_top_->shape(0) &&
+              c < this->blob_top_->shape(1) &&
+              h < this->blob_top_->shape(2) &&
+              w < this->blob_top_->shape(3) ) {
+            EXPECT_EQ(this->blob_bottom_0_->diff_at(n, c, h, w),
+                      this->blob_bottom_0_->data_at(n, c, h, w));
+          } else {
+            EXPECT_EQ(this->blob_bottom_0_->diff_at(n, c, h, w), 0);
+          }
+        }
+      }
+    }
+  }
+}
+
+TYPED_TEST(CropLayerTest, TestGradientNumOffset) {
+  typedef typename TypeParam::Dtype Dtype;
+  LayerParameter layer_param;
+  layer_param.mutable_crop_param()->set_axis(0);
+  layer_param.mutable_crop_param()->add_offset(0);
+  layer_param.mutable_crop_param()->add_offset(1);
+  layer_param.mutable_crop_param()->add_offset(1);
+  layer_param.mutable_crop_param()->add_offset(2);
+  CropLayer<Dtype> layer(layer_param);
+
+  layer.SetUp(this->blob_bottom_vec_0_, this->blob_top_vec_);
+  layer.Forward(this->blob_bottom_vec_0_, this->blob_top_vec_);
+
+  // Copy top data into diff
+  caffe_copy(this->blob_top_->count(), this->blob_top_->cpu_data(),
+             this->blob_top_->mutable_cpu_diff());
+
+  // Do backward pass
+  vector<bool> propagate_down(2, true);
+  layer.Backward(this->blob_top_vec_, propagate_down, this->blob_bottom_vec_0_);
+
+
+  // Check results
+  for (int n = 0; n < this->blob_bottom_0_->num(); ++n) {
+    for (int c = 0; c < this->blob_bottom_0_->channels(); ++c) {
+      for (int h = 0; h < this->blob_bottom_0_->height(); ++h) {
+        for (int w = 0; w < this->blob_bottom_0_->width(); ++w) {
+          if ( 0 <= n && n < 0 + this->blob_top_->shape(0) &&
+              1 <= c && c < 1 + this->blob_top_->shape(1) &&
+              1 <= h && h < 1 + this->blob_top_->shape(2) &&
+              2 <= w && w < 2 + this->blob_top_->shape(3) ) {
+            EXPECT_EQ(this->blob_bottom_0_->diff_at(n, c, h, w),
+                      this->blob_bottom_0_->data_at(n, c, h, w));
+          } else {
+            EXPECT_EQ(this->blob_bottom_0_->diff_at(n, c, h, w), 0);
+          }
+        }
+      }
+    }
+  }
+}
+
+}  // namespace caffe