From fb34bdb40cb901565b87a93c360c4dccc123e902 Mon Sep 17 00:00:00 2001 From: wangchaochaohu Date: Sun, 12 Apr 2020 14:15:52 +0800 Subject: [PATCH] API/OP(fill_constant) error message enhancement (#23584) --- paddle/fluid/framework/parallel_executor.cc | 1 - paddle/fluid/operators/fill_constant_op.cc | 3 +-- paddle/fluid/operators/fill_constant_op.h | 16 ++++++++-------- python/paddle/fluid/layers/tensor.py | 17 +++++++++-------- .../tests/unittests/test_fill_constant_op.py | 14 ++++++++++++-- 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/paddle/fluid/framework/parallel_executor.cc b/paddle/fluid/framework/parallel_executor.cc index 1c4a694a26..851cce56dc 100644 --- a/paddle/fluid/framework/parallel_executor.cc +++ b/paddle/fluid/framework/parallel_executor.cc @@ -789,7 +789,6 @@ void ParallelExecutor::BCastParamsToDevices( FetchResultType ParallelExecutor::Run( const std::vector &fetch_tensors, bool return_merged) { VLOG(3) << "enter ParallelExecutor Run"; - platform::RecordEvent parallel_executor_event("ParallelExecutor::Run"); #ifdef WITH_GPERFTOOLS if (gProfileStarted) { ProfilerFlush(); diff --git a/paddle/fluid/operators/fill_constant_op.cc b/paddle/fluid/operators/fill_constant_op.cc index e9a3c6f90a..8fb12a952f 100644 --- a/paddle/fluid/operators/fill_constant_op.cc +++ b/paddle/fluid/operators/fill_constant_op.cc @@ -22,8 +22,7 @@ class FillConstantOp : public framework::OperatorWithKernel { using framework::OperatorWithKernel::OperatorWithKernel; void InferShape(framework::InferShapeContext* ctx) const override { - PADDLE_ENFORCE_EQ(ctx->HasOutput("Out"), true, - "Output(Out) of FillConstantOp should not be null."); + OP_INOUT_CHECK(ctx->HasOutput("Out"), "Output", "Out", "FillConstant"); auto& shape = ctx->Attrs().Get>("shape"); diff --git a/paddle/fluid/operators/fill_constant_op.h b/paddle/fluid/operators/fill_constant_op.h index 2fde693cbb..d91f50a172 100644 --- a/paddle/fluid/operators/fill_constant_op.h +++ b/paddle/fluid/operators/fill_constant_op.h @@ -49,11 +49,11 @@ inline framework::DDim GetShape(const framework::ExecutionContext &ctx) { auto tensor = shape_tensor_list[i]; PADDLE_ENFORCE_EQ( tensor->dims(), framework::make_ddim({1}), - "ShapeError: If the element type of 'shape' in FillConstantOp is " - "Tensor, " - "the element's shape must be [1]. But received the element's shape " - "is [%s]", - tensor->dims()); + platform::errors::InvalidArgument( + "If the element type of 'shape'(tensor_list type) in " + "FillConstantOp is Tensor, the shape of this Tensor element must " + "be [1]. But received the Tensor element's shape is [%s]", + tensor->dims())); if (platform::is_gpu_place(tensor->place())) { framework::Tensor temp; TensorCopySync(*tensor, platform::CPUPlace(), &temp); @@ -124,9 +124,9 @@ class FillConstantKernel : public framework::OpKernel { tensor = out_var->GetMutable()->mutable_value(); tensor->Resize(shape); } else { - PADDLE_THROW( - "fill constant op's output only" - "supports SelectedRows and LoDTensor"); + PADDLE_THROW(platform::errors::Unimplemented( + "In fill constant Op, the output only supports SelectedRows and " + "LoDTensor.")); } platform::DeviceContextPool &pool = platform::DeviceContextPool::Instance(); diff --git a/python/paddle/fluid/layers/tensor.py b/python/paddle/fluid/layers/tensor.py index 68da9c49ba..c4a553d503 100644 --- a/python/paddle/fluid/layers/tensor.py +++ b/python/paddle/fluid/layers/tensor.py @@ -628,11 +628,18 @@ def fill_constant(shape, dtype, value, force_cpu=False, out=None): out.stop_gradient = True return out - helper = LayerHelper("fill_constant", **locals()) - check_dtype(dtype, 'create data type', + check_dtype(dtype, 'dtype', ['bool', 'float16', 'float32', 'float64', 'int32', 'int64'], 'fill_constant') check_type(shape, 'shape', (Variable, list, tuple), 'fill_constant') + if isinstance(shape, Variable): + check_variable_and_dtype(shape, 'shape', ['int32', 'int64'], + 'fill_constant') + if out is not None: + check_variable_and_dtype(out, 'out', [convert_dtype(dtype)], + 'fill_constant') + + helper = LayerHelper("fill_constant", **locals()) inputs = utils._get_shape_tensor_inputs( inputs=inputs, helper=helper, @@ -642,12 +649,6 @@ def fill_constant(shape, dtype, value, force_cpu=False, out=None): if out is None: out = helper.create_variable_for_type_inference(dtype=dtype) - else: - check_dtype( - dtype, 'create data type', - convert_dtype(out.dtype), 'fill_constant', - '(The create data type in fill_constant must be the same with out data type.)' - ) attrs['dtype'] = out.dtype helper.append_op( type='fill_constant', diff --git a/python/paddle/fluid/tests/unittests/test_fill_constant_op.py b/python/paddle/fluid/tests/unittests/test_fill_constant_op.py index f87c4d4207..ae7639f29c 100644 --- a/python/paddle/fluid/tests/unittests/test_fill_constant_op.py +++ b/python/paddle/fluid/tests/unittests/test_fill_constant_op.py @@ -23,6 +23,7 @@ import paddle.fluid.core as core from paddle.fluid.op import Operator import paddle.fluid as fluid from paddle.fluid import compiler, Program, program_guard +import numpy as np # Situation 1: Attr(shape) is a list(without tensor) @@ -263,12 +264,12 @@ class TestFillConstantOp2_ValueTensor(OpTest): # Test python API class TestFillConstantAPI(unittest.TestCase): def test_api(self): - positive_2_int32 = fluid.layers.fill_constant([1], "int32", 2) + positive_2_int32 = fluid.layers.fill_constant([1], "int32", 2) positive_2_int64 = fluid.layers.fill_constant([1], "int64", 2) + shape_tensor_int32 = fluid.data( name="shape_tensor_int32", shape=[2], dtype="int32") - shape_tensor_int64 = fluid.data( name="shape_tensor_int64", shape=[2], dtype="int64") @@ -349,6 +350,15 @@ class TestFillConstantOpError(unittest.TestCase): dtype='float64', out=x2) + x3 = np.random.randn(100, 100).astype('int32') + self.assertRaises( + TypeError, + fluid.layers.fill_constant, + shape=[100, 100], + value=5, + dtype='float64', + out=x3) + # The argument shape's type of fill_constant_op must be list, tuple or Variable. def test_shape_type(): fluid.layers.fill_constant(shape=1, dtype="float32", value=1)