From c4c8f60bec0f22132369de182673bf0bce3ef29e Mon Sep 17 00:00:00 2001
From: tangwei12 <tangwei12@baidu.com>
Date: Tue, 31 Jul 2018 16:46:41 +0800
Subject: [PATCH 1/6] sum_op selectedRows dim bug fix
---
paddle/fluid/operators/sum_op.h | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/paddle/fluid/operators/sum_op.h b/paddle/fluid/operators/sum_op.h
index 49a4afb3a8..a497c91289 100644
--- a/paddle/fluid/operators/sum_op.h
+++ b/paddle/fluid/operators/sum_op.h
@@ -105,8 +105,15 @@ class SumKernel : public framework::OpKernel<T> {
auto &sel_row = get_selected_row(i);
first_dim += sel_row.rows().size();
}
- auto in_dim =
- framework::vectorize(get_selected_row(N - 1).value().dims());
+
+ std::vector<int64_t> in_dim;
+ for (int i = 0; i < N; i++) {
+ auto &sel_row = get_selected_row(i);
+ if (sel_row.rows().size() > 0) {
+ in_dim = framework::vectorize(sel_row.value().dims());
+ break;
+ }
+ }
in_dim[0] = static_cast<int64_t>(first_dim);
out_value->Resize(framework::make_ddim(in_dim));
From 766ac488acdb5b2034c3d675eb8244693ff7250c Mon Sep 17 00:00:00 2001
From: tangwei12 <tangwei12@baidu.com>
Date: Wed, 1 Aug 2018 16:56:31 +0800
Subject: [PATCH 2/6] sum_op selectedRows dim bug fix
---
paddle/fluid/operators/sum_mkldnn_op.cc | 19 +++++-
paddle/fluid/operators/sum_op.h | 6 +-
.../fluid/tests/unittests/test_sum_op.py | 58 +++++++++++++++++++
3 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/paddle/fluid/operators/sum_mkldnn_op.cc b/paddle/fluid/operators/sum_mkldnn_op.cc
index d2035777ee..c2cf13f7bf 100644
--- a/paddle/fluid/operators/sum_mkldnn_op.cc
+++ b/paddle/fluid/operators/sum_mkldnn_op.cc
@@ -175,18 +175,31 @@ class SumMKLDNNOpKernel : public paddle::framework::OpKernel<T> {
auto& sel_row = get_selected_row(i);
first_dim += sel_row.rows().size();
}
- auto in_dim =
- framework::vectorize(get_selected_row(N - 1).value().dims());
+
+ std::vector<int64_t> in_dim;
+ for (int i = 0; i < N; i++) {
+ auto& sel_row = get_selected_row(i);
+ if (sel_row.rows().size() > 0) {
+ in_dim = framework::vectorize(sel_row.value().dims());
+ break;
+ }
+ }
+
+ if (in_dim.empty()) {
+ in_dim = framework::vectorize(get_selected_row(N - 1).value().dims());
+ }
in_dim[0] = static_cast<int64_t>(first_dim);
out_value->Resize(framework::make_ddim(in_dim));
+ out_value->mutable_data<T>(ctx.GetPlace());
+
// if all the input sparse vars are empty, no need to
// merge these vars.
if (first_dim == 0UL) {
return;
}
- out_value->mutable_data<T>(ctx.GetPlace());
+
math::SelectedRowsAddTo<CPUDeviceContext, T> functor;
int64_t offset = 0;
for (int i = 0; i < N; i++) {
diff --git a/paddle/fluid/operators/sum_op.h b/paddle/fluid/operators/sum_op.h
index a497c91289..da192c6212 100644
--- a/paddle/fluid/operators/sum_op.h
+++ b/paddle/fluid/operators/sum_op.h
@@ -114,16 +114,20 @@ class SumKernel : public framework::OpKernel<T> {
break;
}
}
+ if (in_dim.empty()) {
+ in_dim = framework::vectorize(get_selected_row(N - 1).value().dims());
+ }
+
in_dim[0] = static_cast<int64_t>(first_dim);
out_value->Resize(framework::make_ddim(in_dim));
+ out_value->mutable_data<T>(context.GetPlace());
// if all the input sparse vars are empty, no need to
// merge these vars.
if (first_dim == 0UL) {
return;
}
- out_value->mutable_data<T>(context.GetPlace());
math::SelectedRowsAddTo<DeviceContext, T> functor;
diff --git a/python/paddle/fluid/tests/unittests/test_sum_op.py b/python/paddle/fluid/tests/unittests/test_sum_op.py
index 1d90414e13..3c42607918 100644
--- a/python/paddle/fluid/tests/unittests/test_sum_op.py
+++ b/python/paddle/fluid/tests/unittests/test_sum_op.py
@@ -15,6 +15,8 @@
import unittest
import numpy as np
from op_test import OpTest
+import paddle.fluid.core as core
+from paddle.fluid.op import Operator
class TestSumOp(OpTest):
@@ -40,5 +42,61 @@ class TestSumOp(OpTest):
pass
+class TestSelectedRowsSumOp(OpTest):
+ def check_with_place(self, place):
+ scope = core.Scope()
+ self.check_input_and_optput(scope, place, True, True, True)
+ self.check_input_and_optput(scope, place, False, True, True)
+ self.check_input_and_optput(scope, place, False, False, True)
+ self.check_input_and_optput(scope, place, False, False, False)
+
+ def check_input_and_optput(self, scope, place, w1=False, w2=False,
+ w3=False):
+ W1 = self.create_selected_rows(scope, place, "W1", w1)
+ W2 = self.create_selected_rows(scope, place, "W2", w2)
+ W3 = self.create_selected_rows(scope, place, "W3", w3)
+
+ # create Out Variable
+ out = scope.var('Out').get_selected_rows()
+
+ # create and run sum operator
+ sum_op = Operator("sum", X=["W1", "W2", "W3"], Out='Out')
+ sum_op.run(scope, place)
+
+ trues = 0
+ for w in [w1, w2, w3]:
+ if not w:
+ trues += 1
+
+ self.assertEqual(7 * trues, len(out.rows()))
+
+ def create_selected_rows(self, scope, place, var_name, isEmpty):
+ # create and initialize W Variable
+ if not isEmpty:
+ rows = [0, 1, 2, 3, 4, 5, 6]
+ row_numel = 12
+ else:
+ rows = []
+ row_numel = 12
+
+ var = scope.var(var_name)
+ w_selected_rows = var.get_selected_rows()
+ w_selected_rows.set_height(len(rows))
+ w_selected_rows.set_rows(rows)
+ w_array = np.ones((len(rows), row_numel)).astype("float32")
+ for i in range(len(rows)):
+ w_array[i] *= i
+ w_tensor = w_selected_rows.get_tensor()
+ w_tensor.set(w_array, place)
+
+ return var
+
+ def test_w_is_selected_rows(self):
+ places = [core.CPUPlace()]
+ # currently only support CPU
+ for place in places:
+ self.check_with_place(place)
+
+
if __name__ == "__main__":
unittest.main()
From 26b228e40596bd50c4d590317a44963aa5bf0ea5 Mon Sep 17 00:00:00 2001
From: tangwei12 <tangwei12@baidu.com>
Date: Thu, 16 Aug 2018 16:12:17 +0800
Subject: [PATCH 3/6] remove assignment and add vlog
---
paddle/fluid/operators/sum_mkldnn_op.cc | 11 ++++++-----
paddle/fluid/operators/sum_op.h | 2 +-
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/paddle/fluid/operators/sum_mkldnn_op.cc b/paddle/fluid/operators/sum_mkldnn_op.cc
index c2cf13f7bf..4ccf6b87dc 100644
--- a/paddle/fluid/operators/sum_mkldnn_op.cc
+++ b/paddle/fluid/operators/sum_mkldnn_op.cc
@@ -34,15 +34,15 @@
namespace paddle {
namespace operators {
-using paddle::framework::Tensor;
-using paddle::platform::MKLDNNDeviceContext;
-using paddle::platform::CPUDeviceContext;
using framework::DataLayout;
using mkldnn::memory;
using mkldnn::primitive;
+using mkldnn::reorder;
using mkldnn::stream;
using mkldnn::sum;
-using mkldnn::reorder;
+using paddle::framework::Tensor;
+using paddle::platform::CPUDeviceContext;
+using paddle::platform::MKLDNNDeviceContext;
using platform::to_void_cast;
template <typename T>
@@ -186,8 +186,9 @@ class SumMKLDNNOpKernel : public paddle::framework::OpKernel<T> {
}
if (in_dim.empty()) {
- in_dim = framework::vectorize(get_selected_row(N - 1).value().dims());
+ VLOG(3) << "WARNING: all the inputs are empty"
}
+
in_dim[0] = static_cast<int64_t>(first_dim);
out_value->Resize(framework::make_ddim(in_dim));
diff --git a/paddle/fluid/operators/sum_op.h b/paddle/fluid/operators/sum_op.h
index 22a97fff3a..65ab914057 100644
--- a/paddle/fluid/operators/sum_op.h
+++ b/paddle/fluid/operators/sum_op.h
@@ -115,7 +115,7 @@ class SumKernel : public framework::OpKernel<T> {
}
}
if (in_dim.empty()) {
- in_dim = framework::vectorize(get_selected_row(N - 1).value().dims());
+ VLOG(3) << "WARNING: all the inputs are empty"
}
in_dim[0] = static_cast<int64_t>(first_dim);
From 479a443f68c4295ab7f04a925a925f75c94dc94a Mon Sep 17 00:00:00 2001
From: tangwei12 <tangwei12@baidu.com>
Date: Thu, 16 Aug 2018 20:27:58 +0800
Subject: [PATCH 4/6] name optimized
---
.../fluid/tests/unittests/test_sum_op.py | 23 +++++++++++--------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/python/paddle/fluid/tests/unittests/test_sum_op.py b/python/paddle/fluid/tests/unittests/test_sum_op.py
index 3c42607918..81efd01d50 100644
--- a/python/paddle/fluid/tests/unittests/test_sum_op.py
+++ b/python/paddle/fluid/tests/unittests/test_sum_op.py
@@ -50,11 +50,16 @@ class TestSelectedRowsSumOp(OpTest):
self.check_input_and_optput(scope, place, False, False, True)
self.check_input_and_optput(scope, place, False, False, False)
- def check_input_and_optput(self, scope, place, w1=False, w2=False,
- w3=False):
- W1 = self.create_selected_rows(scope, place, "W1", w1)
- W2 = self.create_selected_rows(scope, place, "W2", w2)
- W3 = self.create_selected_rows(scope, place, "W3", w3)
+ def check_input_and_optput(self,
+ scope,
+ place,
+ w1_has_data=False,
+ w2_has_data=False,
+ w3_has_data=False):
+
+ self.create_selected_rows(scope, place, "W1", w1_has_data)
+ self.create_selected_rows(scope, place, "W2", w2_has_data)
+ self.create_selected_rows(scope, place, "W3", w3_has_data)
# create Out Variable
out = scope.var('Out').get_selected_rows()
@@ -63,12 +68,12 @@ class TestSelectedRowsSumOp(OpTest):
sum_op = Operator("sum", X=["W1", "W2", "W3"], Out='Out')
sum_op.run(scope, place)
- trues = 0
- for w in [w1, w2, w3]:
+ has_data_w_num = 0
+ for w in [w1_has_data, w2_has_data, w3_has_data]:
if not w:
- trues += 1
+ has_data_w_num += 1
- self.assertEqual(7 * trues, len(out.rows()))
+ self.assertEqual(7 * has_data_w_num, len(out.rows()))
def create_selected_rows(self, scope, place, var_name, isEmpty):
# create and initialize W Variable
From ac9ae97001f35fe328c22fbc75e05e2ae927749e Mon Sep 17 00:00:00 2001
From: tangwei12 <tangwei12@baidu.com>
Date: Fri, 17 Aug 2018 11:26:21 +0800
Subject: [PATCH 5/6] code fix
---
paddle/fluid/operators/sum_mkldnn_op.cc | 2 +-
paddle/fluid/operators/sum_op.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/paddle/fluid/operators/sum_mkldnn_op.cc b/paddle/fluid/operators/sum_mkldnn_op.cc
index 4ccf6b87dc..148366a72a 100644
--- a/paddle/fluid/operators/sum_mkldnn_op.cc
+++ b/paddle/fluid/operators/sum_mkldnn_op.cc
@@ -186,7 +186,7 @@ class SumMKLDNNOpKernel : public paddle::framework::OpKernel<T> {
}
if (in_dim.empty()) {
- VLOG(3) << "WARNING: all the inputs are empty"
+ VLOG(3) << "WARNING: all the inputs are empty";
}
in_dim[0] = static_cast<int64_t>(first_dim);
diff --git a/paddle/fluid/operators/sum_op.h b/paddle/fluid/operators/sum_op.h
index 65ab914057..1f5062b799 100644
--- a/paddle/fluid/operators/sum_op.h
+++ b/paddle/fluid/operators/sum_op.h
@@ -115,7 +115,7 @@ class SumKernel : public framework::OpKernel<T> {
}
}
if (in_dim.empty()) {
- VLOG(3) << "WARNING: all the inputs are empty"
+ VLOG(3) << "WARNING: all the inputs are empty";
}
in_dim[0] = static_cast<int64_t>(first_dim);
From b4f52b01d0383a4aa8f7597029a926cf074a07a3 Mon Sep 17 00:00:00 2001
From: tangwei12 <tangwei12@baidu.com>
Date: Sat, 18 Aug 2018 00:27:45 +0800
Subject: [PATCH 6/6] bug fix when all inputs are empty
---
paddle/fluid/operators/sum_mkldnn_op.cc | 3 +++
paddle/fluid/operators/sum_op.h | 5 +++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/paddle/fluid/operators/sum_mkldnn_op.cc b/paddle/fluid/operators/sum_mkldnn_op.cc
index 148366a72a..f9a16ef35e 100644
--- a/paddle/fluid/operators/sum_mkldnn_op.cc
+++ b/paddle/fluid/operators/sum_mkldnn_op.cc
@@ -187,6 +187,9 @@ class SumMKLDNNOpKernel : public paddle::framework::OpKernel<T> {
if (in_dim.empty()) {
VLOG(3) << "WARNING: all the inputs are empty";
+ in_dim = framework::vectorize(get_selected_row(N - 1).value().dims());
+ } else {
+ in_dim[0] = static_cast<int64_t>(first_dim);
}
in_dim[0] = static_cast<int64_t>(first_dim);
diff --git a/paddle/fluid/operators/sum_op.h b/paddle/fluid/operators/sum_op.h
index f1d885b7fd..6dffe527c1 100644
--- a/paddle/fluid/operators/sum_op.h
+++ b/paddle/fluid/operators/sum_op.h
@@ -116,10 +116,11 @@ class SumKernel : public framework::OpKernel<T> {
}
if (in_dim.empty()) {
VLOG(3) << "WARNING: all the inputs are empty";
+ in_dim = framework::vectorize(get_selected_row(N - 1).value().dims());
+ } else {
+ in_dim[0] = static_cast<int64_t>(first_dim);
}
- in_dim[0] = static_cast<int64_t>(first_dim);
-
out_value->Resize(framework::make_ddim(in_dim));
out_value->mutable_data<T>(context.GetPlace());