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.

Sign up for GitHub

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

Fix Crop layer dimension checking to only check cropped dimensions #3993

Merged
merged 3 commits into from
Apr 15, 2016
Merged

Fix Crop layer dimension checking to only check cropped dimensions #3993

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e867e60
[test] CropLayer: test dimensions check to reveal bounds checking bug
shelhamer Apr 15, 2016
75b0d40
[fix] CropLayer: check dimension bounds only for cropped dimensions
shelhamer Apr 15, 2016
00dc3d1
CropLayer: groom comments
shelhamer Apr 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
CropLayer: groom comments
  • Loading branch information
@shelhamer
shelhamer committed Apr 15, 2016
commit 00dc3d1ced4467be00ccc82b8509e4a25d54808d
9 changes: 9 additions & 0 deletions include/caffe/layers/crop_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class CropLayer : public Layer<Dtype> {
vector<int> offsets;

private:
// Recursive copy function.
void crop_copy(const vector<Blob<Dtype>*>& bottom,
const vector<Blob<Dtype>*>& top,
const vector<int>& offsets,
Expand All @@ -53,6 +54,14 @@ class CropLayer : public Layer<Dtype> {
Dtype* dest_data,
bool is_forward);

// Recursive copy function: this is similar to crop_copy() but loops over all
// but the last two dimensions to allow for ND cropping while still relying on
// a CUDA kernel for the innermost two dimensions for performance reasons. An
// alterantive implementation could rely on the kernel more by passing
// offsets, but this is problematic because of its variable length.
// Since in the standard (N,C,W,H) case N,C are usually not cropped a speedup
// could be achieved by not looping the application of the copy_kernel around
// these dimensions.
void crop_copy_gpu(const vector<Blob<Dtype>*>& bottom,
const vector<Blob<Dtype>*>& top,
const vector<int>& offsets,
Expand Down
22 changes: 8 additions & 14 deletions src/caffe/layers/crop_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ 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.
// LayerSetup() handles the number of dimensions; Reshape() handles the sizes.
// bottom[0] supplies the data
// bottom[1] supplies the size
const CropParameter& param = this->layer_param_.crop_param();
Expand All @@ -40,40 +39,35 @@ void CropLayer<Dtype>::Reshape(const vector<Blob<Dtype>*>& bottom,
int input_dim = bottom[0]->num_axes();
const int start_axis = bottom[0]->CanonicalAxisIndex(param.axis());

// initialize all offsets to 0
// Initialize offsets to 0 and the new shape to the current shape of the data.
offsets = vector<int>(input_dim, 0);
// initialize new shape to bottom[0]
vector<int> new_shape(bottom[0]->shape());

// apply crops
// Determine crop offsets and the new shape post-crop.
for (int i = 0; i < input_dim; ++i) {
int crop_offset = 0;
int new_size = bottom[0]->shape(i);
int new_size = bottom[0]->shape(i);
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
// If only one offset is given, all crops have the same offset.
crop_offset = param.offset(0);
} else if (param.offset_size() > 1) {
// crop values specified must be equal to the number of dimensions
// following axis
// For several offsets, the number of offsets must be equal to the
// number of dimensions to crop, that is dimensions after the axis.
crop_offset = param.offset(i - start_axis);
}
// check that the crop and offset are within the dimension bounds
// Check that the crop and offset are within the dimension's bounds.
CHECK_GE(bottom[0]->shape(i) - crop_offset, bottom[1]->shape(i))
<< "the crop for dimension " << i << " is out-of-bounds with "
<< "size " << bottom[1]->shape(i) << " and offset " << crop_offset;
}
// Now set new size and offsets
new_shape[i] = new_size;
offsets[i] = crop_offset;
}
top[0]->Reshape(new_shape);
}

// recursive copy function
template <typename Dtype>
void CropLayer<Dtype>::crop_copy(const vector<Blob<Dtype>*>& bottom,
const vector<Blob<Dtype>*>& top,
Expand Down
9 changes: 0 additions & 9 deletions src/caffe/layers/crop_layer.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,6 @@ __global__ void copy_kernel(const int n, const int height, const int width,
}
}

// recursive copy function, this function is similar to crop_copy but loops
// over all but the last two dimensions. It is implemented this way to allow
// for ND cropping while still relying on a CUDA kernel for the innermost
// two dimensions for performance reasons.
// An alternative way to implement ND cropping relying more on the kernel
// would require passing offsets to the kernel, which is a bit problematic
// because it is of variable length. Since in the standard (N,C,W,H) case
// N,C are usually not cropped a speedup could be achieved by not looping
// the application of the copy_kernel around these dimensions.
template <typename Dtype>
void CropLayer<Dtype>::crop_copy_gpu(const vector<Blob<Dtype>*>& bottom,
const vector<Blob<Dtype>*>& top,
Expand Down