From 8aa5757a290f6b412bda33fd4aebdc9d34eb8a90 Mon Sep 17 00:00:00 2001 From: Hongyu Liu <43953930+phlrain@users.noreply.github.com> Date: Mon, 24 Jun 2019 18:55:31 +0800 Subject: [PATCH] Fix slice op shape=-1 bug (#18107) (#18227) * fix slice op bug; test=develop * fix variabel test bug; test=develop * remove slice while true; test=develop --- paddle/fluid/operators/slice_op.cc | 47 ++++-- paddle/fluid/operators/slice_op.h | 68 ++++++++- python/paddle/fluid/framework.py | 103 +++++++++---- .../fluid/tests/unittests/test_slice_op.py | 140 ++++++++++++++++++ .../fluid/tests/unittests/test_variable.py | 91 +++++++----- 5 files changed, 370 insertions(+), 79 deletions(-) diff --git a/paddle/fluid/operators/slice_op.cc b/paddle/fluid/operators/slice_op.cc index 08b7bf3d1e9..6788b2e2125 100644 --- a/paddle/fluid/operators/slice_op.cc +++ b/paddle/fluid/operators/slice_op.cc @@ -39,21 +39,49 @@ class SliceOp : public framework::OperatorWithKernel { auto axes = ctx->Attrs().Get>("axes"); auto starts = ctx->Attrs().Get>("starts"); auto ends = ctx->Attrs().Get>("ends"); + auto decrease_axis = ctx->Attrs().Get>("decrease_axis"); PADDLE_ENFORCE_EQ(starts.size(), ends.size()); PADDLE_ENFORCE_EQ(starts.size(), axes.size()); int dim_value, start, end; for (size_t i = 0; i < axes.size(); ++i) { dim_value = out_dims[axes[i]]; - start = starts[i] < 0 ? (starts[i] + dim_value) : starts[i]; - end = ends[i] < 0 ? (ends[i] + dim_value) : ends[i]; - start = std::max(start, 0); - end = std::max(end, 0); - start = std::min(start, dim_value); - end = std::min(end, dim_value); - start = std::min(start, end); - out_dims[axes[i]] = end - start; + if (dim_value > 0) { + start = starts[i] < 0 ? (starts[i] + dim_value) : starts[i]; + end = ends[i] < 0 ? (ends[i] + dim_value) : ends[i]; + start = std::max(start, 0); + end = std::max(end, 0); + // start = std::min(start, dim_value); + end = std::min(end, dim_value); + // start = std::min(start, end); + PADDLE_ENFORCE_GT(end, start, "end should greater than start"); + out_dims[axes[i]] = end - start; + } } + + // generate new shape + if (decrease_axis.size() > 0) { + std::vector new_out_shape; + for (size_t i = 0; i < decrease_axis.size(); ++i) { + if (ctx->IsRuntime()) { + PADDLE_ENFORCE_EQ(out_dims[decrease_axis[i]], 1, + "decrease dim should be 1"); + } + out_dims[decrease_axis[i]] = 0; + } + + for (int i = 0; i < out_dims.size(); ++i) { + if (out_dims[i] != 0) { + new_out_shape.push_back(out_dims[i]); + } + } + if (new_out_shape.size() == 0) { + new_out_shape.push_back(1); + } + + out_dims = framework::make_ddim(new_out_shape); + } + ctx->SetOutputDim("Out", out_dims); if (axes[0] != 0) { ctx->ShareLoD("Input", /*->*/ "Out"); @@ -84,7 +112,8 @@ class SliceOpMaker : public framework::OpProtoAndCheckerMaker { AddAttr>( "ends", "(list) Starting indices of corresponding axis in `axes`."); - + AddAttr>("decrease_axis", "(list) decrease_axis") + .SetDefault({}); AddComment(R"DOC( Slice Operator. diff --git a/paddle/fluid/operators/slice_op.h b/paddle/fluid/operators/slice_op.h index f38d08d7640..dadc4f13825 100644 --- a/paddle/fluid/operators/slice_op.h +++ b/paddle/fluid/operators/slice_op.h @@ -55,17 +55,45 @@ class SliceKernel : public framework::OpKernel { *context.template device_context().eigen_device(); auto in = context.Input("Input"); auto out = context.Output("Out"); - out->mutable_data(context.GetPlace()); auto out_dims = out->dims(); auto in_dims = in->dims(); + + // resize out_dims + auto decrease_axis = context.Attr>("decrease_axis"); + if (decrease_axis.size() > 0) { + if (decrease_axis.size() == (size_t)in_dims.size()) { + std::vector vec_origin_out_shape(decrease_axis.size(), 1); + out->Resize(framework::make_ddim(vec_origin_out_shape)); + } else { + std::vector vec_origin_out_shape( + out_dims.size() + decrease_axis.size(), -1); + + for (size_t i = 0; i < decrease_axis.size(); ++i) { + vec_origin_out_shape[decrease_axis[i]] = 1; + } + + int index = 0; + for (size_t i = 0; i < vec_origin_out_shape.size(); ++i) { + if (vec_origin_out_shape[i] == -1) { + vec_origin_out_shape[i] = out_dims[index]; + ++index; + } + } + + out->Resize(framework::make_ddim(vec_origin_out_shape)); + } + } + + out->mutable_data(context.GetPlace()); auto axes = context.Attr>("axes"); auto starts = context.Attr>("starts"); + auto new_out_dims = out->dims(); auto offsets = Eigen::array(); auto extents = Eigen::array(); for (size_t i = 0; i < D; ++i) { offsets[i] = 0; - extents[i] = out_dims[i]; + extents[i] = new_out_dims[i]; } int start; for (size_t i = 0; i < axes.size(); ++i) { @@ -81,8 +109,10 @@ class SliceKernel : public framework::OpKernel { *in); auto out_t = framework::EigenTensor::From( - *out); + *out, new_out_dims); out_t.device(place) = in_t.slice(offsets, extents); + + out->Resize(out_dims); } }; @@ -90,9 +120,7 @@ template class SliceGradKernel : public framework::OpKernel { public: void Compute(const framework::ExecutionContext& ctx) const override { - size_t rank = ctx.Input(framework::GradVarName("Out")) - ->dims() - .size(); + size_t rank = ctx.Input("Input")->dims().size(); switch (rank) { case 1: SliceCompute<1>(ctx); @@ -130,6 +158,32 @@ class SliceGradKernel : public framework::OpKernel { auto axes = context.Attr>("axes"); auto starts = context.Attr>("starts"); + auto decrease_axis = context.Attr>("decrease_axis"); + if (decrease_axis.size() > 0) { + if (decrease_axis.size() == (size_t)in_dims.size()) { + // all dims decrease + std::vector vec_origin_out_shape(decrease_axis.size(), 1); + out_dims = framework::make_ddim(vec_origin_out_shape); + } else { + std::vector vec_origin_out_shape( + out_dims.size() + decrease_axis.size(), -1); + + for (size_t i = 0; i < decrease_axis.size(); ++i) { + vec_origin_out_shape[decrease_axis[i]] = 1; + } + + int index = 0; + for (size_t i = 0; i < vec_origin_out_shape.size(); ++i) { + if (vec_origin_out_shape[i] == -1) { + vec_origin_out_shape[i] = out_dims[index]; + ++index; + } + } + + out_dims = framework::make_ddim(vec_origin_out_shape); + } + } + auto offsets = Eigen::array(); auto extents = Eigen::array(); for (size_t i = 0; i < D; ++i) { @@ -155,7 +209,7 @@ class SliceGradKernel : public framework::OpKernel { *d_input); auto d_out_t = framework::EigenTensor::From( - *d_out); + *d_out, out_dims); d_in_t.device(place) = d_out_t.pad(paddings, 0); } }; diff --git a/python/paddle/fluid/framework.py b/python/paddle/fluid/framework.py index 7e89c4a36ec..1ddc2654d44 100644 --- a/python/paddle/fluid/framework.py +++ b/python/paddle/fluid/framework.py @@ -822,35 +822,84 @@ class Variable(object): Returns: Sliced variable """ - new_var = None - if isinstance(item, tuple): - if len(item) > len(self.shape): - raise IndexError("Too many indexes") - fixedSize = True - for i in range(len(self.shape)): - if self.shape[i] == -1: - fixedSize = False - break - newitem = self._reconstructSliceinfo(item) or item - if fixedSize: - check, info = self._detectContinuesSlice(newitem) - if check: - starts = info[0] - ends = info[1] - axes = [i for i in range(len(starts))] - return self._sliceVar(axes, starts, ends) - else: - new_var = self - for index, o in enumerate(newitem): - new_var = new_var._sliceAndConcatVar(o, index) + if not isinstance(item, tuple): + item = [item] + + decrease_axis = [] + slice_axis = [] + slice_start = [] + slice_end = [] + reverse_axis = [] + + for dim, slice_item in enumerate(item): + if isinstance(slice_item, slice): + start = slice_item.start + end = slice_item.stop + step = slice_item.step if slice_item.step else 1 + + assert (step == 1 or step == -1) + + if step == -1: + reverse_axis.append(dim) + assert (start is None and end is None) + + if start is None and end is None: + continue + + if start is None: + start = 0 + + if end is None: + end = 10000000 + + slice_axis.append(dim) + slice_start.append(start) + slice_end.append(end) else: - new_var = self - for index, o in enumerate(newitem): - new_var = new_var._sliceAndConcatVar(o, index) - else: - new_var = self._sliceAndConcatVar(item, 0) - return new_var + # int + decrease_axis.append(dim) + slice_axis.append(dim) + slice_start.append(slice_item) + slice_end.append(slice_item + 1 + if slice_item != -1 else 10000000) + + out = self + if len(slice_axis) > 0: + # append slice_op here + + slice_out_var = self.block.create_var( + name=unique_name.generate_with_ignorable_key(self.name + + "_slice"), + dtype=self.dtype) + + self.block.append_op( + type="slice", + inputs={'Input': [out]}, + outputs={'Out': [slice_out_var]}, + attrs={ + 'axes': slice_axis, + 'starts': slice_start, + 'ends': slice_end, + 'decrease_axis': decrease_axis + }) + + out = slice_out_var + + if len(reverse_axis) > 0: + reverse_out_var = self.block.create_var( + name=unique_name.generate_with_ignorable_key(self.name + + "_slice_reverse"), + dtype=self.dtype) + self.block.append_op( + type="reverse", + inputs={'X': out}, + outputs={'Out': [reverse_out_var]}, + attrs={'axis': reverse_axis}) + + out = reverse_out_var + + return out def get_all_op_protos(): diff --git a/python/paddle/fluid/tests/unittests/test_slice_op.py b/python/paddle/fluid/tests/unittests/test_slice_op.py index aefd8cb6d3a..062eabd3c8f 100644 --- a/python/paddle/fluid/tests/unittests/test_slice_op.py +++ b/python/paddle/fluid/tests/unittests/test_slice_op.py @@ -46,6 +46,146 @@ class TestSliceOp(OpTest): self.check_grad(['Input'], 'Out', max_relative_error=0.006) +class TestSliceOp_decs_dim(OpTest): + def setUp(self): + self.op_type = "slice" + self.config() + self.inputs = {'Input': self.input} + self.outputs = {'Out': self.out} + self.attrs = { + 'axes': self.axes, + 'starts': self.starts, + 'ends': self.ends, + 'decrease_axis': self.decrease_axis, + } + + def config(self): + self.input = np.random.random([3, 4, 5, 6]).astype("float32") + self.starts = [1, 0, 2] + self.ends = [2, 3, 4] + self.axes = [0, 1, 2] + self.decrease_axis = [0] + self.out = self.input[1, 0:3, 2:4, :] + + def test_check_output(self): + self.check_output() + + def test_check_grad_normal(self): + self.check_grad(['Input'], 'Out', max_relative_error=0.006) + + +class TestSliceOp_decs_dim_2(OpTest): + def setUp(self): + self.op_type = "slice" + self.config() + self.inputs = {'Input': self.input} + self.outputs = {'Out': self.out} + self.attrs = { + 'axes': self.axes, + 'starts': self.starts, + 'ends': self.ends, + 'decrease_axis': self.decrease_axis, + } + + def config(self): + self.input = np.random.random([3, 4, 5, 6]).astype("float32") + self.starts = [1, 0, 2] + self.ends = [2, 1, 4] + self.axes = [0, 1, 2] + self.decrease_axis = [0, 1] + self.out = self.input[1, 0, 2:4, :] + + def test_check_output(self): + self.check_output() + + def test_check_grad_normal(self): + self.check_grad(['Input'], 'Out', max_relative_error=0.006) + + +class TestSliceOp_decs_dim_3(OpTest): + def setUp(self): + self.op_type = "slice" + self.config() + self.inputs = {'Input': self.input} + self.outputs = {'Out': self.out} + self.attrs = { + 'axes': self.axes, + 'starts': self.starts, + 'ends': self.ends, + 'decrease_axis': self.decrease_axis, + } + + def config(self): + self.input = np.random.random([3, 4, 5, 6]).astype("float32") + self.starts = [-1, 0, 2] + self.ends = [1000000, 1, 4] + self.axes = [0, 1, 2] + self.decrease_axis = [0, 1] + self.out = self.input[-1, 0, 2:4, :] + + def test_check_output(self): + self.check_output() + + def test_check_grad_normal(self): + self.check_grad(['Input'], 'Out', max_relative_error=0.006) + + +class TestSliceOp_decs_dim_5(OpTest): + def setUp(self): + self.op_type = "slice" + self.config() + self.inputs = {'Input': self.input} + self.outputs = {'Out': self.out} + self.attrs = { + 'axes': self.axes, + 'starts': self.starts, + 'ends': self.ends, + 'decrease_axis': self.decrease_axis, + } + + def config(self): + self.input = np.random.random([3, 4, 5, 6]).astype("float32") + self.starts = [-1] + self.ends = [1000000] + self.axes = [3] + self.decrease_axis = [3] + self.out = self.input[:, :, :, -1] + + def test_check_output(self): + self.check_output() + + def test_check_grad_normal(self): + self.check_grad(['Input'], 'Out', max_relative_error=0.006) + + +class TestSliceOp_decs_dim_6(OpTest): + def setUp(self): + self.op_type = "slice" + self.config() + self.inputs = {'Input': self.input} + self.outputs = {'Out': self.out} + self.attrs = { + 'axes': self.axes, + 'starts': self.starts, + 'ends': self.ends, + 'decrease_axis': self.decrease_axis, + } + + def config(self): + self.input = np.random.random([3, 4, 5, 6]).astype("float32") + self.starts = [0, 1, 2, 3] + self.ends = [1, 2, 3, 4] + self.axes = [0, 1, 2, 3] + self.decrease_axis = [0, 1, 2, 3] + self.out = self.input[0, 1, 2, 3:4] + + def test_check_output(self): + self.check_output() + + def test_check_grad_normal(self): + self.check_grad(['Input'], 'Out', max_relative_error=0.006) + + class TestCase1(TestSliceOp): def config(self): self.input = np.random.random([3, 4, 5, 6]).astype("float32") diff --git a/python/paddle/fluid/tests/unittests/test_variable.py b/python/paddle/fluid/tests/unittests/test_variable.py index a6c43bb8373..d72511ee0f0 100644 --- a/python/paddle/fluid/tests/unittests/test_variable.py +++ b/python/paddle/fluid/tests/unittests/test_variable.py @@ -17,6 +17,7 @@ from __future__ import print_function import unittest from paddle.fluid.framework import default_main_program, Program, convert_np_dtype_to_dtype_, in_dygraph_mode import paddle.fluid as fluid +import paddle.fluid.layers as layers import paddle.fluid.core as core import numpy as np @@ -67,19 +68,24 @@ class TestVariable(unittest.TestCase): for i in range(3): nw = w[i] - self.assertEqual((1, 100, 100), nw.shape) + self.assertEqual((100, 100), nw.shape) nw = w[:] self.assertEqual((784, 100, 100), nw.shape) - nw = w[:, :, ...] + nw = w[:, :] self.assertEqual((784, 100, 100), nw.shape) - nw = w[::2, ::2, :] - self.assertEqual((392, 50, 100), nw.shape) + nw = w[:, :, -1] + self.assertEqual((784, 100), nw.shape) - nw = w[::-2, ::-2, :] - self.assertEqual((392, 50, 100), nw.shape) + nw = w[1, 1, 1] + + self.assertEqual(len(nw.shape), 1) + self.assertEqual(nw.shape[0], 1) + + nw = w[:, :, :-1] + self.assertEqual((784, 100, 99), nw.shape) self.assertEqual(0, nw.lod_level) @@ -94,18 +100,23 @@ class TestVariable(unittest.TestCase): var1 = var[0, 1, 1] var2 = var[1:] var3 = var[0:1] - var4 = var[..., ] - var5 = var[2::-2] - var6 = var[1, 1:, 1:] - var7 = var[1, ..., 1:] - var8 = var[1, ...] + var4 = var[::-1] + var5 = var[1, 1:, 1:] var_reshape = fluid.layers.reshape(var, [3, -1, 3]) - var9 = var_reshape[1, ..., 2] - var10 = var_reshape[:, :, -1] + var6 = var_reshape[:, :, -1] + var7 = var[:, :, :-1] + var8 = var[:1, :1, :1] + var9 = var[:-1, :-1, :-1] + var10 = var[::-1, :1, :-1] + var11 = var[:-1, ::-1, -1:] + var12 = var[1:2, 2:, ::-1] + var13 = var[2:10, 2:, -2:-1] + var14 = var[1:-1, 0:2, ::-1] + var15 = var[::-1, ::-1, ::-1] x = fluid.layers.data(name='x', shape=[13], dtype='float32') y = fluid.layers.fc(input=x, size=1, act=None) - var11 = y[:, 0] + y_1 = y[:, 0] feeder = fluid.DataFeeder(place=place, feed_list=[x]) data = [] data.append((np.random.randint(10, size=[13]).astype('float32'))) @@ -115,28 +126,38 @@ class TestVariable(unittest.TestCase): feed=feeder.feed([data]), fetch_list=[ var, var1, var2, var3, var4, var5, var6, - var7, var8, var9, var10, var11 + var7, var8, var9, var10, var11, var12, + var13, var14, var15 ]) - self.assertTrue((np.array(local_out[1]) == np.array(tensor_array[ - 0, 1, 1])).all()) - self.assertTrue((np.array(local_out[2]) == np.array(tensor_array[ - 1:])).all()) - self.assertTrue((np.array(local_out[3]) == np.array(tensor_array[ - 0:1])).all()) - self.assertTrue((np.array(local_out[4]) == np.array( - tensor_array[..., ])).all()) - self.assertTrue((np.array(local_out[5]) == np.array(tensor_array[ - 2::-2])).all()) - self.assertTrue((np.array(local_out[6]) == np.array(tensor_array[ - 1, 1:, 1:])).all()) - self.assertTrue((np.array(local_out[7]) == np.array(tensor_array[ - 1, ..., 1:])).all()) - self.assertTrue((np.array(local_out[8]) == np.array(tensor_array[ - 1, ...])).all()) - self.assertEqual(local_out[9].shape, (1, 3, 1)) - self.assertEqual(local_out[10].shape, (3, 3, 1)) - self.assertEqual(local_out[11].shape, (1, 1)) + self.assertTrue( + np.array_equal(local_out[1], tensor_array[0, 1, 1:2])) + self.assertTrue(np.array_equal(local_out[2], tensor_array[1:])) + self.assertTrue(np.array_equal(local_out[3], tensor_array[0:1])) + self.assertTrue(np.array_equal(local_out[4], tensor_array[::-1])) + self.assertTrue( + np.array_equal(local_out[5], tensor_array[1, 1:, 1:])) + self.assertTrue( + np.array_equal(local_out[6], + tensor_array.reshape((3, -1, 3))[:, :, -1])) + self.assertTrue( + np.array_equal(local_out[7], tensor_array[:, :, :-1])) + self.assertTrue( + np.array_equal(local_out[8], tensor_array[:1, :1, :1])) + self.assertTrue( + np.array_equal(local_out[9], tensor_array[:-1, :-1, :-1])) + self.assertTrue( + np.array_equal(local_out[10], tensor_array[::-1, :1, :-1])) + self.assertTrue( + np.array_equal(local_out[11], tensor_array[:-1, ::-1, -1:])) + self.assertTrue( + np.array_equal(local_out[12], tensor_array[1:2, 2:, ::-1])) + self.assertTrue( + np.array_equal(local_out[13], tensor_array[2:10, 2:, -2:-1])) + self.assertTrue( + np.array_equal(local_out[14], tensor_array[1:-1, 0:2, ::-1])) + self.assertTrue( + np.array_equal(local_out[15], tensor_array[::-1, ::-1, ::-1])) def test_slice(self): place = fluid.CPUPlace() @@ -148,12 +169,10 @@ class TestVariable(unittest.TestCase): def _tostring(self): b = default_main_program().current_block() w = b.create_var(dtype="float64", lod_level=0) - print(w) self.assertTrue(isinstance(str(w), str)) if core.is_compiled_with_cuda(): wc = b.create_var(dtype="int", lod_level=0) - print(wc) self.assertTrue(isinstance(str(wc), str)) def test_tostring(self): -- GitLab