From: Jeff Donahue Date: Sat, 12 Jul 2014 05:52:02 +0000 (-0700) Subject: Disallow in-place computation in SPLIT layer -- has strange effects in X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f50b233a2607cac2472c1799c332eabf0093c4ae;p=platform%2Fupstream%2Fcaffe.git Disallow in-place computation in SPLIT layer -- has strange effects in backward pass when input into a loss. --- diff --git a/include/caffe/vision_layers.hpp b/include/caffe/vision_layers.hpp index 31df136..d2d9ab3 100644 --- a/include/caffe/vision_layers.hpp +++ b/include/caffe/vision_layers.hpp @@ -230,7 +230,7 @@ class LRNLayer : public Layer { Blob power_output_; vector*> power_top_vec_; shared_ptr > product_layer_; - Blob product_data_input_; + Blob product_input_; vector*> product_bottom_vec_; }; diff --git a/src/caffe/layers/lrn_layer.cpp b/src/caffe/layers/lrn_layer.cpp index c76ca95..e81a32b 100644 --- a/src/caffe/layers/lrn_layer.cpp +++ b/src/caffe/layers/lrn_layer.cpp @@ -26,7 +26,7 @@ void LRNLayer::LayerSetUp(const vector*>& bottom, { // Set up split_layer_ to use inputs in the numerator and denominator. split_top_vec_.clear(); - split_top_vec_.push_back(bottom[0]); + split_top_vec_.push_back(&product_input_); split_top_vec_.push_back(&square_input_); LayerParameter split_param; split_layer_.reset(new SplitLayer(split_param)); @@ -76,7 +76,7 @@ void LRNLayer::LayerSetUp(const vector*>& bottom, // Set up a product_layer_ to compute outputs by multiplying inputs by the // inverse demoninator computed by the power layer. product_bottom_vec_.clear(); - product_bottom_vec_.push_back(bottom[0]); + product_bottom_vec_.push_back(&product_input_); product_bottom_vec_.push_back(&power_output_); LayerParameter product_param; EltwiseParameter* eltwise_param = product_param.mutable_eltwise_param(); diff --git a/src/caffe/layers/split_layer.cpp b/src/caffe/layers/split_layer.cpp index c223d47..3bed347 100644 --- a/src/caffe/layers/split_layer.cpp +++ b/src/caffe/layers/split_layer.cpp @@ -11,12 +11,13 @@ void SplitLayer::LayerSetUp(const vector*>& bottom, vector*>* top) { count_ = bottom[0]->count(); for (int i = 0; i < top->size(); ++i) { - // Allow the 0th top blob to be 'in-place', but no others. - if (i == 0 && (*top)[i] == bottom[0]) { - continue; - } else { - CHECK_NE((*top)[i], bottom[0]) << "Only 0th top blob may be in place."; - } + // Do not allow in-place computation in the SplitLayer. Instead, share data + // by reference in the forward pass, and keep separate diff allocations in + // the backward pass. (Technically, it should be possible to share the diff + // blob of the first split output with the input, but this seems to cause + // some strange effects in practice...) + CHECK_NE((*top)[i], bottom[0]) << this->type_name() << " Layer does not " + "allow in-place computation."; (*top)[i]->Reshape(bottom[0]->num(), bottom[0]->channels(), bottom[0]->height(), bottom[0]->width()); CHECK_EQ(count_, (*top)[i]->count()); @@ -34,14 +35,18 @@ void SplitLayer::Forward_cpu(const vector*>& bottom, template void SplitLayer::Backward_cpu(const vector*>& top, const vector& propagate_down, vector*>* bottom) { - if (propagate_down[0]) { - (*bottom)[0]->ShareDiff(*top[0]); - // Add remaining top blob diffs. + if (!propagate_down[0]) { return; } + if (top.size() == 1) { + caffe_copy(count_, top[0]->cpu_diff(), (*bottom)[0]->mutable_cpu_diff()); + return; + } + caffe_add(count_, top[0]->cpu_diff(), top[1]->cpu_diff(), + (*bottom)[0]->mutable_cpu_diff()); + // Add remaining top blob diffs. + for (int i = 2; i < top.size(); ++i) { + const Dtype* top_diff = top[i]->cpu_diff(); Dtype* bottom_diff = (*bottom)[0]->mutable_cpu_diff(); - for (int i = 1; i < top.size(); ++i) { - const Dtype* top_diff = top[i]->cpu_diff(); - caffe_axpy(count_, Dtype(1.), top_diff, bottom_diff); - } + caffe_axpy(count_, Dtype(1.), top_diff, bottom_diff); } } diff --git a/src/caffe/layers/split_layer.cu b/src/caffe/layers/split_layer.cu index 2d2b3c2..fcc0917 100644 --- a/src/caffe/layers/split_layer.cu +++ b/src/caffe/layers/split_layer.cu @@ -17,14 +17,18 @@ void SplitLayer::Forward_gpu(const vector*>& bottom, template void SplitLayer::Backward_gpu(const vector*>& top, const vector& propagate_down, vector*>* bottom) { - if (propagate_down[0]) { - (*bottom)[0]->ShareDiff(*top[0]); - // Add remaining top blob diffs. + if (!propagate_down[0]) { return; } + if (top.size() == 1) { + caffe_copy(count_, top[0]->gpu_diff(), (*bottom)[0]->mutable_gpu_diff()); + return; + } + caffe_gpu_add(count_, top[0]->gpu_diff(), top[1]->gpu_diff(), + (*bottom)[0]->mutable_gpu_diff()); + // Add remaining top blob diffs. + for (int i = 2; i < top.size(); ++i) { + const Dtype* top_diff = top[i]->gpu_diff(); Dtype* bottom_diff = (*bottom)[0]->mutable_gpu_diff(); - for (int i = 1; i < top.size(); ++i) { - const Dtype* top_diff = top[i]->gpu_diff(); - caffe_gpu_axpy(count_, Dtype(1.), top_diff, bottom_diff); - } + caffe_gpu_axpy(count_, Dtype(1.), top_diff, bottom_diff); } } diff --git a/src/caffe/test/test_split_layer.cpp b/src/caffe/test/test_split_layer.cpp index 448e13c..92a3ec7 100644 --- a/src/caffe/test/test_split_layer.cpp +++ b/src/caffe/test/test_split_layer.cpp @@ -76,19 +76,6 @@ TYPED_TEST(SplitLayerTest, Test) { } } -TYPED_TEST(SplitLayerTest, TestInPlace) { - typedef typename TypeParam::Dtype Dtype; - LayerParameter layer_param; - SplitLayer layer(layer_param); - this->blob_top_vec_[0] = this->blob_bottom_vec_[0]; - layer.SetUp(this->blob_bottom_vec_, &(this->blob_top_vec_)); - layer.Forward(this->blob_bottom_vec_, &(this->blob_top_vec_)); - for (int i = 0; i < this->blob_bottom_->count(); ++i) { - Dtype bottom_value = this->blob_bottom_->cpu_data()[i]; - EXPECT_EQ(bottom_value, this->blob_top_b_->cpu_data()[i]); - } -} - TYPED_TEST(SplitLayerTest, TestGradient) { typedef typename TypeParam::Dtype Dtype; LayerParameter layer_param; @@ -98,16 +85,6 @@ TYPED_TEST(SplitLayerTest, TestGradient) { &(this->blob_top_vec_)); } -TYPED_TEST(SplitLayerTest, TestGradientInPlace) { - typedef typename TypeParam::Dtype Dtype; - LayerParameter layer_param; - SplitLayer layer(layer_param); - GradientChecker checker(1e-2, 1e-2); - this->blob_top_vec_[0] = this->blob_bottom_vec_[0]; - checker.CheckGradientEltwise(&layer, &(this->blob_bottom_vec_), - &(this->blob_top_vec_)); -} - class SplitLayerInsertionTest : public ::testing::Test { protected: @@ -124,12 +101,12 @@ class SplitLayerInsertionTest : public ::testing::Test { output_param_string, &expected_output_param)); NetParameter actual_output_param; InsertSplits(input_param, &actual_output_param); - CHECK_EQ(expected_output_param.DebugString(), + EXPECT_EQ(expected_output_param.DebugString(), actual_output_param.DebugString()); // Also test idempotence. NetParameter double_split_insert_param; InsertSplits(actual_output_param, &double_split_insert_param); - CHECK_EQ(actual_output_param.DebugString(), + EXPECT_EQ(actual_output_param.DebugString(), double_split_insert_param.DebugString()); } }; @@ -624,7 +601,7 @@ TEST_F(SplitLayerInsertionTest, TestLossInsertion) { " name: 'data_data_0_split' " " type: SPLIT " " bottom: 'data' " - " top: 'data' " + " top: 'data_data_0_split_0' " " top: 'data_data_0_split_1' " "} " "layers: { " @@ -639,14 +616,14 @@ TEST_F(SplitLayerInsertionTest, TestLossInsertion) { " } " " } " " param: 'unsharedweights1' " - " bottom: 'data' " + " bottom: 'data_data_0_split_0' " " top: 'innerproduct1' " "} " "layers: { " " name: 'innerproduct1_innerproduct1_0_split' " " type: SPLIT " " bottom: 'innerproduct1' " - " top: 'innerproduct1' " + " top: 'innerproduct1_innerproduct1_0_split_0' " " top: 'innerproduct1_innerproduct1_0_split_1' " " loss_weight: 2.5 " " loss_weight: 0 " @@ -726,14 +703,14 @@ TEST_F(SplitLayerInsertionTest, TestInsertion) { " name: 'data_data_0_split' " " type: SPLIT " " bottom: 'data' " - " top: 'data' " + " top: 'data_data_0_split_0' " " top: 'data_data_0_split_1' " " top: 'data_data_0_split_2' " "} " "layers: { " " name: 'innerprod1' " " type: INNER_PRODUCT " - " bottom: 'data' " + " bottom: 'data_data_0_split_0' " " top: 'innerprod1' " "} " "layers: { " @@ -746,7 +723,7 @@ TEST_F(SplitLayerInsertionTest, TestInsertion) { " name: 'innerprod2_innerprod2_0_split' " " type: SPLIT " " bottom: 'innerprod2' " - " top: 'innerprod2' " + " top: 'innerprod2_innerprod2_0_split_0' " " top: 'innerprod2_innerprod2_0_split_1' " "} " "layers: { " @@ -759,7 +736,7 @@ TEST_F(SplitLayerInsertionTest, TestInsertion) { " name: 'loss1' " " type: EUCLIDEAN_LOSS " " bottom: 'innerprod1' " - " bottom: 'innerprod2' " + " bottom: 'innerprod2_innerprod2_0_split_0' " "} " "layers: { " " name: 'loss2' " @@ -827,26 +804,26 @@ TEST_F(SplitLayerInsertionTest, TestInsertionTwoTop) { " name: 'data_data_0_split' " " type: SPLIT " " bottom: 'data' " - " top: 'data' " + " top: 'data_data_0_split_0' " " top: 'data_data_0_split_1' " "} " "layers: { " " name: 'label_data_1_split' " " type: SPLIT " " bottom: 'label' " - " top: 'label' " + " top: 'label_data_1_split_0' " " top: 'label_data_1_split_1' " "} " "layers: { " " name: 'innerprod1' " " type: INNER_PRODUCT " - " bottom: 'data' " + " bottom: 'data_data_0_split_0' " " top: 'innerprod1' " "} " "layers: { " " name: 'innerprod2' " " type: INNER_PRODUCT " - " bottom: 'label' " + " bottom: 'label_data_1_split_0' " " top: 'innerprod2' " "} " "layers: { " @@ -913,13 +890,13 @@ TEST_F(SplitLayerInsertionTest, TestInputInsertion) { " name: 'data_input_0_split' " " type: SPLIT " " bottom: 'data' " - " top: 'data' " + " top: 'data_input_0_split_0' " " top: 'data_input_0_split_1' " "} " "layers: { " " name: 'innerprod1' " " type: INNER_PRODUCT " - " bottom: 'data' " + " bottom: 'data_input_0_split_0' " " top: 'innerprod1' " "} " "layers: { " @@ -988,13 +965,13 @@ TEST_F(SplitLayerInsertionTest, TestWithInPlace) { " name: 'data_data_0_split' " " type: SPLIT " " bottom: 'data' " - " top: 'data' " + " top: 'data_data_0_split_0' " " top: 'data_data_0_split_1' " "} " "layers: { " " name: 'innerprod1' " " type: INNER_PRODUCT " - " bottom: 'data' " + " bottom: 'data_data_0_split_0' " " top: 'innerprod1' " "} " "layers: { " @@ -1007,13 +984,13 @@ TEST_F(SplitLayerInsertionTest, TestWithInPlace) { " name: 'innerprod1_relu1_0_split' " " type: SPLIT " " bottom: 'innerprod1' " - " top: 'innerprod1' " + " top: 'innerprod1_relu1_0_split_0' " " top: 'innerprod1_relu1_0_split_1' " "} " "layers: { " " name: 'innerprod2' " " type: INNER_PRODUCT " - " bottom: 'innerprod1' " + " bottom: 'innerprod1_relu1_0_split_0' " " top: 'innerprod2' " "} " "layers: { " diff --git a/src/caffe/util/insert_splits.cpp b/src/caffe/util/insert_splits.cpp index e917e35..f20efda 100644 --- a/src/caffe/util/insert_splits.cpp +++ b/src/caffe/util/insert_splits.cpp @@ -135,11 +135,6 @@ string SplitLayerName(const string& layer_name, const string& blob_name, string SplitBlobName(const string& layer_name, const string& blob_name, const int blob_idx, const int split_idx) { - // 0th split top blob is given the same name as the bottom blob so that - // computation is done 'in-place', saving a bit of time and memory. - if (split_idx == 0) { - return blob_name; - } ostringstream split_blob_name; split_blob_name << blob_name << "_" << layer_name << "_" << blob_idx << "_split_" << split_idx;