From 08d70cbac490eb68e0d457b05bd99134da481d2f Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Wed, 13 Nov 2019 17:07:16 +0800 Subject: [PATCH 01/17] add unused input vars check for OpWithKernel, test=develop --- paddle/fluid/framework/CMakeLists.txt | 6 +- paddle/fluid/framework/operator.cc | 17 +++++ paddle/fluid/framework/operator.h | 13 ++++ paddle/fluid/framework/unused_var_check.cc | 74 ++++++++++++++++++++++ paddle/fluid/framework/unused_var_check.h | 33 ++++++++++ python/paddle/fluid/__init__.py | 2 +- 6 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 paddle/fluid/framework/unused_var_check.cc create mode 100644 paddle/fluid/framework/unused_var_check.h diff --git a/paddle/fluid/framework/CMakeLists.txt b/paddle/fluid/framework/CMakeLists.txt index 79af44edfed499..ac67a8f4f2cba9 100644 --- a/paddle/fluid/framework/CMakeLists.txt +++ b/paddle/fluid/framework/CMakeLists.txt @@ -83,6 +83,7 @@ endif() cc_test(var_type_traits_test SRCS var_type_traits_test.cc DEPS var_type_traits) cc_library(scope SRCS scope.cc DEPS glog threadpool xxhash var_type_traits) + cc_library(scope_pool SRCS scope_pool.cc DEPS scope) cc_test(scope_test SRCS scope_test.cc DEPS scope) cc_test(variable_test SRCS variable_test.cc DEPS tensor var_type_traits) @@ -126,8 +127,11 @@ cc_test(no_need_buffer_vars_inference_test SRCS no_need_buffer_vars_inference_te cc_library(transfer_scope_cache SRCS transfer_scope_cache.cc DEPS scope framework_proto device_context) cc_library(op_kernel_type SRCS op_kernel_type.cc DEPS device_context place) + +cc_library(unused_var_check SRCS unused_var_check.cc DEPS glog no_need_buffer_vars_inference) + cc_library(operator SRCS operator.cc DEPS op_info device_context tensor scope glog trainer_desc_proto data_feed_proto - shape_inference data_transform lod_tensor profiler transfer_scope_cache op_kernel_type op_call_stack) + shape_inference data_transform lod_tensor profiler transfer_scope_cache op_kernel_type op_call_stack unused_var_check) cc_test(operator_test SRCS operator_test.cc DEPS operator op_registry device_context) cc_test(operator_exception_test SRCS operator_exception_test.cc DEPS operator op_registry device_context) diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index 22a62d806fa60a..5d8f880911eba4 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -28,6 +28,7 @@ limitations under the License. */ #include "paddle/fluid/framework/operator.h" #include "paddle/fluid/framework/shape_inference.h" #include "paddle/fluid/framework/transfer_scope_cache.h" +#include "paddle/fluid/framework/unused_var_check.h" #include "paddle/fluid/framework/var_type.h" #include "paddle/fluid/platform/profiler.h" @@ -428,6 +429,10 @@ bool ExecutionContext::HasOutput(const std::string& name) const { } const Variable* ExecutionContext::InputVar(const std::string& name) const { + if (FLAGS_enable_unused_var_check) { + VLOG(6) << "InputVar: " << name; + GetThreadLocalUsedVarNameSet()->insert(name); + } auto it = ctx_.inputs.find(name); if (it == ctx_.inputs.end()) return nullptr; @@ -457,6 +462,10 @@ const Tensor* ExecutionContext::Input(const std::string& name) const { template <> const std::vector ExecutionContext::MultiInput( const std::string& name) const { + if (FLAGS_enable_unused_var_check) { + VLOG(6) << "MultiInput: " << name; + GetThreadLocalUsedVarNameSet()->insert(name); + } auto it = ctx_.inputs.find(name); if (it == ctx_.inputs.end()) { return {}; @@ -860,6 +869,10 @@ void OperatorWithKernel::RunImpl(const Scope& scope, const platform::Place& place) const { // To reduce the elapsed time of HasAttr, we use bool variable to record the // result of HasAttr. + if (FLAGS_enable_unused_var_check) { + GetThreadLocalUsedVarNameSet()->clear(); + } + if (!enable_cache_runtime_context_ && HasAttr(kEnableCacheRuntimeContext)) enable_cache_runtime_context_ = true; if (!all_kernels_must_compute_runtime_shape_ && @@ -879,6 +892,10 @@ void OperatorWithKernel::RunImpl(const Scope& scope, } RunImpl(scope, place, runtime_ctx_.get()); } + + if (FLAGS_enable_unused_var_check) { + CheckUnusedVar(*this); + } } void OperatorWithKernel::RunImpl(const Scope& scope, diff --git a/paddle/fluid/framework/operator.h b/paddle/fluid/framework/operator.h index 6a9af6af31588e..ba5172fe390fea 100644 --- a/paddle/fluid/framework/operator.h +++ b/paddle/fluid/framework/operator.h @@ -35,6 +35,7 @@ limitations under the License. */ #include "paddle/fluid/framework/scope.h" #include "paddle/fluid/framework/selected_rows.h" #include "paddle/fluid/framework/tensor.h" +#include "paddle/fluid/framework/unused_var_check.h" #include "paddle/fluid/memory/malloc.h" #include "paddle/fluid/platform/device_context.h" #include "paddle/fluid/platform/variant.h" @@ -268,6 +269,10 @@ class ExecutionContext { const std::vector MultiInputVar( const std::string& name) const { + if (FLAGS_enable_unused_var_check) { + VLOG(6) << "MultiInputVar: " << name; + GetThreadLocalUsedVarNameSet()->insert(name); + } auto it = ctx_.inputs.find(name); if (it == ctx_.inputs.end()) { return {}; @@ -298,6 +303,10 @@ class ExecutionContext { template const std::vector MultiInput(const std::string& name) const { + if (FLAGS_enable_unused_var_check) { + VLOG(6) << "MultiInput: " << name; + GetThreadLocalUsedVarNameSet()->insert(name); + } auto it = ctx_.inputs.find(name); if (it == ctx_.inputs.end()) { return {}; @@ -349,6 +358,10 @@ class ExecutionContext { //! Get actual name vector for this input. const std::vector& Inputs(const std::string& name) const { + if (FLAGS_enable_unused_var_check) { + VLOG(6) << "Inputs: " << name; + GetThreadLocalUsedVarNameSet()->insert(name); + } return op_.Inputs(name); } diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc new file mode 100644 index 00000000000000..dcb8d49d813735 --- /dev/null +++ b/paddle/fluid/framework/unused_var_check.cc @@ -0,0 +1,74 @@ +/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. */ + +#include +#include + +#include +#include +#include +#include "paddle/fluid/framework/operator.h" +#include "paddle/fluid/framework/unused_var_check.h" +#include "paddle/fluid/platform/enforce.h" + +DEFINE_bool(enable_unused_var_check, true, + "Checking whether operator contains unused inputs, " + "especially for grad operator. It should be in unittest."); + +namespace paddle { +namespace framework { + +std::unordered_set *GetThreadLocalUsedVarNameSet() { + thread_local std::unordered_set used_var_name_set; + return &used_var_name_set; +} + +void CheckUnusedVar(const OperatorBase &op) { + auto *used_set = GetThreadLocalUsedVarNameSet(); + std::vector unsed_input_var_names; + auto &inferer = op.Info().NoNeedBufferVarsInferer(); + std::unordered_set no_need_buffer_ins = {}; + if (inferer) { + no_need_buffer_ins = inferer(op.Inputs(), op.Outputs(), op.Attrs()); + } + + for (auto &pair : op.Inputs()) { + // skip no need buffer vars declared + if (no_need_buffer_ins.count(pair.first) != 0) { + VLOG(6) << op.Type() << " " << pair.first; + continue; + } + // skip XShape, since it contains no tensor + if (pair.first == "XShape") { + continue; + } + if (pair.second.size() != 0 && used_set->count(pair.first) == 0) { + unsed_input_var_names.emplace_back(pair.first); + } + } + if (!unsed_input_var_names.empty()) { + std::string err_msg = "Operator " + op.Type() + " has input(s) not uesed: "; + for (auto &in_var_name : unsed_input_var_names) { + err_msg += in_var_name; + err_msg += ", "; + } + err_msg += "please remove it from inputs or register NoNeedBufferVars!"; + LOG(ERROR) << err_msg; + PADDLE_ENFORCE_EQ(unsed_input_var_names.size(), 0, + platform::errors::PermissionDenied("%s", err_msg)); + } +} + +} // namespace framework +} // namespace paddle diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h new file mode 100644 index 00000000000000..229ef1a31b1ad5 --- /dev/null +++ b/paddle/fluid/framework/unused_var_check.h @@ -0,0 +1,33 @@ +/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. */ + +#pragma once + +#include +#include + +#include +#include +#include "paddle/fluid/framework/operator.h" + +DECLARE_bool(enable_unused_var_check); + +namespace paddle { +namespace framework { + +std::unordered_set* GetThreadLocalUsedVarNameSet(); +void CheckUnusedVar(const OperatorBase& op); + +} // namespace framework +} // namespace paddle diff --git a/python/paddle/fluid/__init__.py b/python/paddle/fluid/__init__.py index 6336911da9b63d..dff8a790db9122 100644 --- a/python/paddle/fluid/__init__.py +++ b/python/paddle/fluid/__init__.py @@ -168,7 +168,7 @@ def __bootstrap__(): 'print_sub_graph_dir', 'pe_profile_fname', 'inner_op_parallelism', 'enable_parallel_graph', 'fuse_parameter_groups_size', 'multiple_of_cupti_buffer_size', 'fuse_parameter_memory_size', - 'tracer_profile_fname', 'dygraph_debug' + 'tracer_profile_fname', 'dygraph_debug', 'enable_unused_var_check' ] if 'Darwin' not in sysstr: read_env_flags.append('use_pinned_memory') From dc41961e5872f14893ec52015440f102f993f194 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Tue, 19 Nov 2019 15:08:43 +0800 Subject: [PATCH 02/17] remove unused vars in some ops, test=develop --- paddle/fluid/operators/batch_norm_op.cc | 1 - paddle/fluid/operators/center_loss_op.cc | 1 - .../detection/roi_perspective_transform_op.cc | 2 -- .../fused/fusion_seqpool_cvm_concat_op.cc | 3 -- .../operators/hierarchical_sigmoid_op.cc | 1 - paddle/fluid/operators/instance_norm_op.h | 1 - paddle/fluid/operators/lrn_op.cc | 26 ++++++++++++++--- paddle/fluid/operators/metrics/auc_op.cc | 4 +-- paddle/fluid/operators/nce_op.cc | 25 +++++++++++++++++ paddle/fluid/operators/prelu_op.cc | 28 ++++++++++++++++--- 10 files changed, 73 insertions(+), 19 deletions(-) diff --git a/paddle/fluid/operators/batch_norm_op.cc b/paddle/fluid/operators/batch_norm_op.cc index bc0f16a96621a7..616e09294a9bcf 100644 --- a/paddle/fluid/operators/batch_norm_op.cc +++ b/paddle/fluid/operators/batch_norm_op.cc @@ -626,7 +626,6 @@ class BatchNormGradMaker : public framework::SingleGradOpMaker { op->SetInput(framework::GradVarName("Y"), this->OutputGrad("Y")); op->SetInput("Scale", this->Input("Scale")); - op->SetInput("Bias", this->Input("Bias")); op->SetInput("SavedMean", this->Output("SavedMean")); op->SetInput("SavedVariance", this->Output("SavedVariance")); diff --git a/paddle/fluid/operators/center_loss_op.cc b/paddle/fluid/operators/center_loss_op.cc index f0c0a5e619f7c9..02f1b349f1fb49 100644 --- a/paddle/fluid/operators/center_loss_op.cc +++ b/paddle/fluid/operators/center_loss_op.cc @@ -134,7 +134,6 @@ class CenterLossOpGradMaker : public framework::SingleGradOpMaker { retv->SetType("center_loss_grad"); retv->SetInput(framework::GradVarName("Loss"), this->OutputGrad("Loss")); retv->SetInput("SampleCenterDiff", this->Output("SampleCenterDiff")); - retv->SetInput("X", this->Input("X")); retv->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); retv->SetAttrMap(this->Attrs()); diff --git a/paddle/fluid/operators/detection/roi_perspective_transform_op.cc b/paddle/fluid/operators/detection/roi_perspective_transform_op.cc index b220ea540a899e..6440cabf3769f0 100644 --- a/paddle/fluid/operators/detection/roi_perspective_transform_op.cc +++ b/paddle/fluid/operators/detection/roi_perspective_transform_op.cc @@ -632,8 +632,6 @@ class ROIPerspectiveTransformGradMaker op->SetType("roi_perspective_transform_grad"); op->SetInput("X", this->Input("X")); op->SetInput("ROIs", this->Input("ROIs")); - op->SetInput("Out2InIdx", this->Output("Out2InIdx")); - op->SetInput("Out2InWeights", this->Output("Out2InWeights")); op->SetInput(framework::GradVarName("Out"), this->OutputGrad("Out")); op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); op->SetAttrMap(this->Attrs()); diff --git a/paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc b/paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc index f64e4f134d62f1..633620278225b5 100644 --- a/paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc +++ b/paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc @@ -57,9 +57,6 @@ framework::OpKernelType FusionSeqPoolCVMConcatOp::GetExpectedKernelType( void FusionSeqPoolCVMConcatOpMaker::Make() { AddInput("X", "(LoDTensor) Input tensors of this operator.").AsDuplicable(); - AddInput("CVM", - "(Tensor), a 2-D Tensor with shape [N x 2], where N is the batch " - "size, 2 is show and click."); AddOutput("Out", "(LoDTensor) Output tensor of concat operator."); AddAttr("pooltype", "(string, default 'SUM') some of the pooling " diff --git a/paddle/fluid/operators/hierarchical_sigmoid_op.cc b/paddle/fluid/operators/hierarchical_sigmoid_op.cc index 58e380183f1df6..e76c833bbcd0b7 100644 --- a/paddle/fluid/operators/hierarchical_sigmoid_op.cc +++ b/paddle/fluid/operators/hierarchical_sigmoid_op.cc @@ -195,7 +195,6 @@ class HierarchicalSigmoidGradMaker : public framework::SingleGradOpMaker { // Inputs: X, W, Label, PathTable, PathCode, PreOut, Out@GRAD op->SetInput("X", this->Input("X")); op->SetInput("W", this->Input("W")); - op->SetInput("Bias", this->Input("Bias")); op->SetInput("Label", this->Input("Label")); op->SetInput("PathTable", this->Input("PathTable")); op->SetInput("PathCode", this->Input("PathCode")); diff --git a/paddle/fluid/operators/instance_norm_op.h b/paddle/fluid/operators/instance_norm_op.h index 9ff6f09ad0121a..a9c011850bbf0f 100644 --- a/paddle/fluid/operators/instance_norm_op.h +++ b/paddle/fluid/operators/instance_norm_op.h @@ -87,7 +87,6 @@ class InstanceNormGradMaker : public framework::SingleGradOpMaker { op->SetInput(framework::GradVarName("Y"), this->OutputGrad("Y")); op->SetInput("Scale", this->Input("Scale")); - op->SetInput("Bias", this->Input("Bias")); op->SetInput("SavedMean", this->Output("SavedMean")); op->SetInput("SavedVariance", this->Output("SavedVariance")); diff --git a/paddle/fluid/operators/lrn_op.cc b/paddle/fluid/operators/lrn_op.cc index 33a418f9f44f35..1840016f29d8ff 100644 --- a/paddle/fluid/operators/lrn_op.cc +++ b/paddle/fluid/operators/lrn_op.cc @@ -13,6 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ #include "paddle/fluid/operators/lrn_op.h" +#include #include #include #include "paddle/fluid/operators/math/blas.h" @@ -320,14 +321,31 @@ class LRNOpGrad : public framework::OperatorWithKernel { layout_, library_); } }; + +template +class LRNGradOpMaker : public framework::SingleGradOpMaker { + public: + using framework::SingleGradOpMaker::SingleGradOpMaker; + std::unique_ptr Apply() const override { + auto* op = new T(); + op->SetType(this->ForwardOpType() + "_grad"); + op->SetInput("X", this->Input("X")); + op->SetInput("Out", this->Output("Out")); + op->SetInput("MidOut", this->Output("MidOut")); + op->SetInput(framework::GradVarName("Out"), this->OutputGrad("Out")); + op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); + op->SetAttrMap(this->Attrs()); + return std::unique_ptr(op); + } +}; + } // namespace operators } // namespace paddle namespace ops = paddle::operators; -REGISTER_OPERATOR( - lrn, ops::LRNOp, ops::LRNOpMaker, - paddle::framework::DefaultGradOpMaker, - paddle::framework::DefaultGradOpMaker); +REGISTER_OPERATOR(lrn, ops::LRNOp, ops::LRNOpMaker, + ops::LRNGradOpMaker, + ops::LRNGradOpMaker); REGISTER_OPERATOR(lrn_grad, ops::LRNOpGrad); REGISTER_OP_CPU_KERNEL( diff --git a/paddle/fluid/operators/metrics/auc_op.cc b/paddle/fluid/operators/metrics/auc_op.cc index 90734307accc6c..9d2845da08da3f 100644 --- a/paddle/fluid/operators/metrics/auc_op.cc +++ b/paddle/fluid/operators/metrics/auc_op.cc @@ -75,8 +75,8 @@ class AucOpMaker : public framework::OpProtoAndCheckerMaker { "shape: [batch_size, 1]"); // TODO(typhoonzero): support weight input - AddInput("StatPos", "Statistic value when label = 1"); - AddInput("StatNeg", "Statistic value when label = 0"); + // AddInput("StatPos", "Statistic value when label = 1"); + // AddInput("StatNeg", "Statistic value when label = 0"); AddOutput("AUC", "A scalar representing the " diff --git a/paddle/fluid/operators/nce_op.cc b/paddle/fluid/operators/nce_op.cc index b6f68e3bee7da0..2805e172ffaa22 100644 --- a/paddle/fluid/operators/nce_op.cc +++ b/paddle/fluid/operators/nce_op.cc @@ -301,6 +301,31 @@ class NCEOpGradVarTypeInference : public framework::VarTypeInference { } }; +template +class NCEGradOpMaker : public framework::SingleGradOpMaker { + public: + using framework::SingleGradOpMaker::SingleGradOpMaker; + std::unique_ptr Apply() const override { + auto *op = new T(); + op->SetType(this->ForwardOpType() + "_grad"); + op->SetInput("Input", this->Input("Input")); + op->SetInput("Label", this->Input("Label")); + op->SetInput("Weight", this->Input("Weight")); + op->SetInput("SampleLogits", this->Output("SampleLogits")); + op->SetInput("SampleLabels", this->Output("SampleLabels")); + op->SetInput("SampleWeight", this->Input("SampleWeight")); + op->SetInput("CustomDistProbs", this->Input("CustomDistProbs")); + op->SetInput("CustomDistAlias", this->Input("CustomDistAlias")); + op->SetInput("CustomDistAliasProbs", this->Input("CustomDistAliasProbs")); + op->SetInput(framework::GradVarName("Cost"), this->OutputGrad("Cost")); + op->SetOutput(framework::GradVarName("Input"), this->InputGrad("Input")); + op->SetOutput(framework::GradVarName("Bias"), this->InputGrad("Bias")); + op->SetOutput(framework::GradVarName("Weight"), this->InputGrad("Weight")); + op->SetAttrMap(this->Attrs()); + return std::unique_ptr(op); + } +}; + } // namespace operators } // namespace paddle diff --git a/paddle/fluid/operators/prelu_op.cc b/paddle/fluid/operators/prelu_op.cc index 0d63558d97f002..7127177e35b60c 100644 --- a/paddle/fluid/operators/prelu_op.cc +++ b/paddle/fluid/operators/prelu_op.cc @@ -10,6 +10,7 @@ See the License for the specific language governing permissions and limitations under the License. */ #include "paddle/fluid/operators/prelu_op.h" +#include #include namespace paddle { @@ -130,15 +131,34 @@ class PReluGradOp : public framework::OperatorWithKernel { } }; +template +class PReluGradOpMaker : public framework::SingleGradOpMaker { + public: + using framework::SingleGradOpMaker::SingleGradOpMaker; + + protected: + std::unique_ptr Apply() const override { + std::unique_ptr op(new T()); + op->SetType("prelu_grad"); + op->SetInput("X", this->Input("X")); + op->SetInput("Alpha", this->Input("Alpha")); + op->SetInput(framework::GradVarName("Out"), this->OutputGrad("Out")); + op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); + op->SetOutput(framework::GradVarName("Alpha"), this->InputGrad("Alpha")); + op->SetAttrMap(this->Attrs()); + + return op; + } +}; + } // namespace operators } // namespace paddle namespace ops = paddle::operators; -REGISTER_OPERATOR( - prelu, ops::PReluOp, ops::PReluOpMaker, - paddle::framework::DefaultGradOpMaker, - paddle::framework::DefaultGradOpMaker); +REGISTER_OPERATOR(prelu, ops::PReluOp, ops::PReluOpMaker, + ops::PReluGradOpMaker, + ops::PReluGradOpMaker); REGISTER_OPERATOR(prelu_grad, ops::PReluGradOp); REGISTER_OP_CPU_KERNEL( prelu, ops::PReluKernel); From 032ef2fe668f38184728990ea438ae5d6b916a9f Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Wed, 20 Nov 2019 17:34:56 +0800 Subject: [PATCH 03/17] fix batch_norm, test=develop --- paddle/fluid/framework/unused_var_check.cc | 2 - paddle/fluid/operators/batch_norm_op.cc | 48 +++++++++----------- paddle/fluid/operators/batch_norm_op.h | 9 ++++ paddle/fluid/operators/sync_batch_norm_op.cc | 38 ---------------- 4 files changed, 30 insertions(+), 67 deletions(-) diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index dcb8d49d813735..a7b55b7be1da0b 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -65,8 +65,6 @@ void CheckUnusedVar(const OperatorBase &op) { } err_msg += "please remove it from inputs or register NoNeedBufferVars!"; LOG(ERROR) << err_msg; - PADDLE_ENFORCE_EQ(unsed_input_var_names.size(), 0, - platform::errors::PermissionDenied("%s", err_msg)); } } diff --git a/paddle/fluid/operators/batch_norm_op.cc b/paddle/fluid/operators/batch_norm_op.cc index 616e09294a9bcf..fb43832d7f61b6 100644 --- a/paddle/fluid/operators/batch_norm_op.cc +++ b/paddle/fluid/operators/batch_norm_op.cc @@ -614,36 +614,30 @@ class BatchNormGradKernel }; template -class BatchNormGradMaker : public framework::SingleGradOpMaker { - public: - using framework::SingleGradOpMaker::SingleGradOpMaker; - - protected: - std::unique_ptr Apply() const override { - auto *op = new T(); - op->SetType(this->ForwardOpType() + "_grad"); - op->SetInput("X", this->Input("X")); - op->SetInput(framework::GradVarName("Y"), this->OutputGrad("Y")); - - op->SetInput("Scale", this->Input("Scale")); - op->SetInput("SavedMean", this->Output("SavedMean")); - op->SetInput("SavedVariance", this->Output("SavedVariance")); - - // used when setting use_global_stats True during training - if (boost::get(this->GetAttr("use_global_stats"))) { - op->SetInput("Mean", this->Output("MeanOut")); - op->SetInput("Variance", this->Output("VarianceOut")); - } +std::unique_ptr BatchNormGradMaker::Apply() const { + auto *op = new T(); + op->SetType(this->ForwardOpType() + "_grad"); + op->SetInput("X", this->Input("X")); + op->SetInput(framework::GradVarName("Y"), this->OutputGrad("Y")); + + op->SetInput("Scale", this->Input("Scale")); + op->SetInput("SavedMean", this->Output("SavedMean")); + op->SetInput("SavedVariance", this->Output("SavedVariance")); + + // used when setting use_global_stats True during training + if (boost::get(this->GetAttr("use_global_stats"))) { + op->SetInput("Mean", this->Output("MeanOut")); + op->SetInput("Variance", this->Output("VarianceOut")); + } - op->SetAttrMap(this->Attrs()); + op->SetAttrMap(this->Attrs()); - op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); - op->SetOutput(framework::GradVarName("Scale"), this->InputGrad("Scale")); - op->SetOutput(framework::GradVarName("Bias"), this->InputGrad("Bias")); + op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); + op->SetOutput(framework::GradVarName("Scale"), this->InputGrad("Scale")); + op->SetOutput(framework::GradVarName("Bias"), this->InputGrad("Bias")); - return std::unique_ptr(op); - } -}; + return std::unique_ptr(op); +} } // namespace operators } // namespace paddle diff --git a/paddle/fluid/operators/batch_norm_op.h b/paddle/fluid/operators/batch_norm_op.h index 4746a1d2cf1ade..9678e2d3f2b4d3 100644 --- a/paddle/fluid/operators/batch_norm_op.h +++ b/paddle/fluid/operators/batch_norm_op.h @@ -64,6 +64,15 @@ class BatchNormOpMaker : public framework::OpProtoAndCheckerMaker { void Make() override; }; +template +class BatchNormGradMaker : public framework::SingleGradOpMaker { + public: + using framework::SingleGradOpMaker::SingleGradOpMaker; + + protected: + std::unique_ptr Apply() const override; +}; + class BatchNormOpInferVarType : public framework::PassInDtypeAndVarTypeToOutput { protected: diff --git a/paddle/fluid/operators/sync_batch_norm_op.cc b/paddle/fluid/operators/sync_batch_norm_op.cc index a134541dd78066..82204fe37d9914 100644 --- a/paddle/fluid/operators/sync_batch_norm_op.cc +++ b/paddle/fluid/operators/sync_batch_norm_op.cc @@ -14,44 +14,6 @@ limitations under the License. */ #include "paddle/fluid/operators/batch_norm_op.h" -namespace paddle { -namespace operators { -template -class BatchNormGradMaker : public framework::SingleGradOpMaker { - public: - using framework::SingleGradOpMaker::SingleGradOpMaker; - - protected: - std::unique_ptr Apply() const override { - auto *op = new T(); - op->SetType(this->ForwardOpType() + "_grad"); - op->SetInput("X", this->Input("X")); - op->SetInput(framework::GradVarName("Y"), this->OutputGrad("Y")); - - op->SetInput("Scale", this->Input("Scale")); - op->SetInput("Bias", this->Input("Bias")); - op->SetInput("SavedMean", this->Output("SavedMean")); - op->SetInput("SavedVariance", this->Output("SavedVariance")); - - // used when setting use_global_stats True during training - if (boost::get(this->GetAttr("use_global_stats"))) { - op->SetInput("Mean", this->Output("MeanOut")); - op->SetInput("Variance", this->Output("VarianceOut")); - } - - op->SetAttrMap(this->Attrs()); - - op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); - op->SetOutput(framework::GradVarName("Scale"), this->InputGrad("Scale")); - op->SetOutput(framework::GradVarName("Bias"), this->InputGrad("Bias")); - - return std::unique_ptr(op); - } -}; - -} // namespace operators -} // namespace paddle - namespace ops = paddle::operators; REGISTER_OPERATOR(sync_batch_norm, ops::BatchNormOp, ops::BatchNormOpMaker, ops::BatchNormOpInferVarType, From 7aff8563335fb60261c9a367cb0c2b44b6fd6162 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Wed, 20 Nov 2019 20:32:48 +0800 Subject: [PATCH 04/17] add white list, test=develop --- ...enerate_op_use_unused_inputs_white_list.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tools/generate_op_use_unused_inputs_white_list.py diff --git a/tools/generate_op_use_unused_inputs_white_list.py b/tools/generate_op_use_unused_inputs_white_list.py new file mode 100644 index 00000000000000..79ed8b944934ce --- /dev/null +++ b/tools/generate_op_use_unused_inputs_white_list.py @@ -0,0 +1,42 @@ +# Copyright (c) 2019 PaddlePaddle Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +os.environ['CUDA_VISIBLE_DEVICES'] = '' + +import paddle.fluid as fluid +import sys + +white_list = [ + 'fill_zeros_like', + 'batch_norm', + 'crop', + 'cvm', + 'op_with_kernel', + 'dgc_momentum', + 'fake_quantize_range_abs_max', + 'flatten_grad', + 'batch_norm_grad', + 'squeeze_grad', +] + +if len(sys.argv) != 2: + print( + 'Usage: python generate_op_use_unused_inputs_white_list.py [filepath]') + sys.exit(1) + +with open(sys.argv[1], 'w') as f: + for op in white_list: + f.write(op + '\n') From e69b905be73698b76e00aa85500068e889048a0e Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Wed, 20 Nov 2019 21:45:00 +0800 Subject: [PATCH 05/17] add CI check for white list, test=develop --- paddle/fluid/framework/unused_var_check.cc | 2 +- paddle/scripts/paddle_build.sh | 7 +++++-- tools/check_api_approvals.sh | 11 +++++++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index a7b55b7be1da0b..114a0b17174604 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -64,7 +64,7 @@ void CheckUnusedVar(const OperatorBase &op) { err_msg += ", "; } err_msg += "please remove it from inputs or register NoNeedBufferVars!"; - LOG(ERROR) << err_msg; + VLOG(1) << err_msg; } } diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index 5221ab1ec5b724..bafb94661d2ed3 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -436,8 +436,11 @@ EOF fi # TODO: jiabin need to refine this part when these tests fixed on mac - ctest --output-on-failure -j $2 - + # ctest --output-on-failure -j $2 + # NOTE(cql): output logs of LOG(ERROR) to check unused vars. + export FLAGS_enable_unused_var_check=1 + export GLOG_vmodule=unused_var_check=2 + ctest -VV -j $2 -O log.txt paddle version fi } diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index a959f23a0176f4..a1614bfc8e782b 100644 --- a/tools/check_api_approvals.sh +++ b/tools/check_api_approvals.sh @@ -25,7 +25,8 @@ API_FILES=("CMakeLists.txt" "python/paddle/fluid/parallel_executor.py" "python/paddle/fluid/framework.py" "python/paddle/fluid/backward.py" - "paddle/fluid/operators/distributed/send_recv.proto.in") + "paddle/fluid/operators/distributed/send_recv.proto.in" + "tools/generate_op_use_unused_inputs_white_list.py") approval_line=`curl -H "Authorization: token ${GITHUB_API_TOKEN}" https://api.github.com/repos/PaddlePaddle/Paddle/pulls/${GIT_PR_ID}/reviews?per_page=10000` git_files=`git diff --numstat upstream/$BRANCH| wc -l` @@ -67,7 +68,7 @@ for API_FILE in ${API_FILES[*]}; do echo "checking ${API_FILE} change, PR: ${GIT_PR_ID}, changes: ${API_CHANGE}" if [ "${API_CHANGE}" ] && [ "${GIT_PR_ID}" != "" ]; then # NOTE: per_page=10000 should be ok for all cases, a PR review > 10000 is not human readable. - # approval_user_list: XiaoguangHu01 46782768,Xreki 12538138,luotao1 6836917,sneaxiy 32832641,qingqing01 7845005,guoshengCS 14105589,heavengate 12605721,kuke 3064195,Superjomn 328693,lanxianghit 47554610,cyj1986 39645414,hutuxian 11195205,frankwhzhang 20274488,nepeplwu 45024560,Dianhai 38231817,JiabinYang 22361972,chenwhql 22561442,seiriosPlus 5442383,gongweibao 10721757,saxon-zh 2870059,Boyan-Liu 2870059. + # approval_user_list: XiaoguangHu01 46782768,Xreki 12538138,luotao1 6836917,sneaxiy 32832641,qingqing01 7845005,guoshengCS 14105589,heavengate 12605721,kuke 3064195,Superjomn 328693,lanxianghit 47554610,cyj1986 39645414,hutuxian 11195205,frankwhzhang 20274488,nepeplwu 45024560,Dianhai 38231817,JiabinYang 22361972,chenwhql 22561442,zhiqiu 6888866,seiriosPlus 5442383,gongweibao 10721757,saxon-zh 2870059,Boyan-Liu 2870059. if [ "${API_FILE}" == "paddle/fluid/op_use_default_grad_op_maker.spec" ];then APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 32832641 6836917` elif [ "${API_FILE}" == "CMakeLists.txt" ];then @@ -78,6 +79,8 @@ for API_FILE in ${API_FILES[*]}; do APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 6836917 22361972` elif [ "${API_FILE}" == "paddle/fluid/operators/distributed/send_recv.proto.in" ];then APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 10721757 5442383` + elif [ "${API_FILE}" == "tools/generate_op_use_unused_inputs_white_list.py" ];then + APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 6888866 32832641 6836917` else APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 3048612 46782768 12538138 6836917 32832641` fi @@ -103,6 +106,10 @@ for API_FILE in ${API_FILES[*]}; do failed_num=`expr $failed_num + 1` echo_line="You must have one RD (gongweibao or seiriosPlus) approval for the paddle/fluid/operators/distributed/send_recv.proto.in, which manages the environment variables.\n" echo_list=(${echo_list[@]}$failed_num "." $echo_line) + elif [ "${API_FILE}" == "tools/generate_op_use_unused_inputs_white_list.py" ];then + failed_num=`expr $failed_num + 1` + echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the tools/generate_op_use_unused_inputs_white_list.py, which manages the white list of oerators that have unused input variables.\n" + echo_list=(${echo_list[@]}$failed_num "." $echo_line) else failed_num=`expr $failed_num + 1` echo_line="You must have one RD (XiaoguangHu01,Xreki,luotao1,sneaxiy) approval for ${API_FILE}, which manages the underlying code for fluid.\n" From afc8da5af3c9a623ffdee7150be29778d8008c3d Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Thu, 21 Nov 2019 15:29:00 +0800 Subject: [PATCH 06/17] :ove white list to c++, test=develop --- paddle/fluid/framework/operator.cc | 2 +- paddle/fluid/framework/unused_var_check.cc | 21 ++++++---- paddle/fluid/framework/unused_var_check.h | 15 ++++++- paddle/fluid/operators/flatten_op.cc | 5 ++- paddle/fluid/operators/nce_op.cc | 27 ------------ paddle/fluid/operators/squeeze_op.cc | 6 ++- tools/check_api_approvals.sh | 8 ++-- ...enerate_op_use_unused_inputs_white_list.py | 42 ------------------- 8 files changed, 41 insertions(+), 85 deletions(-) delete mode 100644 tools/generate_op_use_unused_inputs_white_list.py diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index 5d8f880911eba4..15ac2ec854fc1c 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -894,7 +894,7 @@ void OperatorWithKernel::RunImpl(const Scope& scope, } if (FLAGS_enable_unused_var_check) { - CheckUnusedVar(*this); + CheckUnusedVar(*this, scope); } } diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index 114a0b17174604..c207be1708aa74 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -34,7 +34,11 @@ std::unordered_set *GetThreadLocalUsedVarNameSet() { return &used_var_name_set; } -void CheckUnusedVar(const OperatorBase &op) { +void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { + // skip op in white list + if (op_has_unsed_vars_white_list.count(op.Type()) != 0) { + return; + } auto *used_set = GetThreadLocalUsedVarNameSet(); std::vector unsed_input_var_names; auto &inferer = op.Info().NoNeedBufferVarsInferer(); @@ -49,12 +53,15 @@ void CheckUnusedVar(const OperatorBase &op) { VLOG(6) << op.Type() << " " << pair.first; continue; } - // skip XShape, since it contains no tensor - if (pair.first == "XShape") { - continue; - } - if (pair.second.size() != 0 && used_set->count(pair.first) == 0) { - unsed_input_var_names.emplace_back(pair.first); + if (used_set->count(pair.first) == 0) { + for (auto &in_var_name : pair.second) { + auto *in_var = scope.FindVar(in_var_name); + auto &tensor = in_var->Get(); + if (in_var->IsInitialized() && tensor.IsInitialized()) { + unsed_input_var_names.emplace_back(pair.first); + break; + } + } } } if (!unsed_input_var_names.empty()) { diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h index 229ef1a31b1ad5..434d06ccc4298c 100644 --- a/paddle/fluid/framework/unused_var_check.h +++ b/paddle/fluid/framework/unused_var_check.h @@ -26,8 +26,21 @@ DECLARE_bool(enable_unused_var_check); namespace paddle { namespace framework { +const std::unordered_set op_has_unsed_vars_white_list = { + "batch_norm", + "batch_norm_grad", + "crop", + "cvm", + "dgc_momentum", + "fake_quantize_range_abs_max", + "fill_zeros_like", + "reshape2_grad_grad", + "reshape2_grad", + "gru_grad", + "op_with_kernel"}; + std::unordered_set* GetThreadLocalUsedVarNameSet(); -void CheckUnusedVar(const OperatorBase& op); +void CheckUnusedVar(const OperatorBase& op, const Scope& scope); } // namespace framework } // namespace paddle diff --git a/paddle/fluid/operators/flatten_op.cc b/paddle/fluid/operators/flatten_op.cc index 320e132d22236a..59509686de0d31 100644 --- a/paddle/fluid/operators/flatten_op.cc +++ b/paddle/fluid/operators/flatten_op.cc @@ -234,6 +234,8 @@ DECLARE_INPLACE_OP_INFERER(FlattenOpInplaceInToOut, {"X", "Out"}); DECLARE_INPLACE_OP_INFERER(FlattenGradInplaceinToOut, {framework::GradVarName("Out"), framework::GradVarName("X")}); +DECLARE_NO_NEED_BUFFER_VARS_INFERENCE(FlattenGradNoNeedBufferVarsInference, + "Out"); } // namespace operators } // namespace paddle @@ -245,7 +247,8 @@ REGISTER_OPERATOR( paddle::framework::DefaultGradOpMaker, ops::FlattenOpInplaceInToOut); REGISTER_OPERATOR(flatten_grad, ops::FlattenGradOp, - ops::FlattenGradInplaceinToOut); + ops::FlattenGradInplaceinToOut, + ops::FlattenGradNoNeedBufferVarsInference); REGISTER_OPERATOR(flatten2, ops::Flatten2Op, ops::Flatten2OpMaker, ops::Flatten2GradOpMaker, diff --git a/paddle/fluid/operators/nce_op.cc b/paddle/fluid/operators/nce_op.cc index 2805e172ffaa22..ec600243a2f411 100644 --- a/paddle/fluid/operators/nce_op.cc +++ b/paddle/fluid/operators/nce_op.cc @@ -222,9 +222,7 @@ class NCEGradOpMaker : public framework::SingleGradOpMaker { op->SetType(this->ForwardOpType() + "_grad"); op->SetInput("Input", this->Input("Input")); op->SetInput("Label", this->Input("Label")); - op->SetInput("Bias", this->Input("Bias")); op->SetInput("Weight", this->Input("Weight")); - op->SetInput("Cost", this->Output("Cost")); op->SetInput("SampleLogits", this->Output("SampleLogits")); op->SetInput("SampleLabels", this->Output("SampleLabels")); op->SetInput("SampleWeight", this->Input("SampleWeight")); @@ -301,31 +299,6 @@ class NCEOpGradVarTypeInference : public framework::VarTypeInference { } }; -template -class NCEGradOpMaker : public framework::SingleGradOpMaker { - public: - using framework::SingleGradOpMaker::SingleGradOpMaker; - std::unique_ptr Apply() const override { - auto *op = new T(); - op->SetType(this->ForwardOpType() + "_grad"); - op->SetInput("Input", this->Input("Input")); - op->SetInput("Label", this->Input("Label")); - op->SetInput("Weight", this->Input("Weight")); - op->SetInput("SampleLogits", this->Output("SampleLogits")); - op->SetInput("SampleLabels", this->Output("SampleLabels")); - op->SetInput("SampleWeight", this->Input("SampleWeight")); - op->SetInput("CustomDistProbs", this->Input("CustomDistProbs")); - op->SetInput("CustomDistAlias", this->Input("CustomDistAlias")); - op->SetInput("CustomDistAliasProbs", this->Input("CustomDistAliasProbs")); - op->SetInput(framework::GradVarName("Cost"), this->OutputGrad("Cost")); - op->SetOutput(framework::GradVarName("Input"), this->InputGrad("Input")); - op->SetOutput(framework::GradVarName("Bias"), this->InputGrad("Bias")); - op->SetOutput(framework::GradVarName("Weight"), this->InputGrad("Weight")); - op->SetAttrMap(this->Attrs()); - return std::unique_ptr(op); - } -}; - } // namespace operators } // namespace paddle diff --git a/paddle/fluid/operators/squeeze_op.cc b/paddle/fluid/operators/squeeze_op.cc index 76817f302c6030..58b052680318d0 100644 --- a/paddle/fluid/operators/squeeze_op.cc +++ b/paddle/fluid/operators/squeeze_op.cc @@ -274,7 +274,8 @@ DECLARE_INPLACE_OP_INFERER(SequeezeInplaceInferer, {"X", "Out"}); DECLARE_INPLACE_OP_INFERER(SequeezeGradInplaceInferer, {framework::GradVarName("Out"), framework::GradVarName("X")}); - +DECLARE_NO_NEED_BUFFER_VARS_INFERENCE(SqueezeGradNoNeedBufferVarsInference, + "Out"); } // namespace operators } // namespace paddle @@ -283,7 +284,8 @@ REGISTER_OPERATOR( squeeze, ops::SqueezeOp, ops::SqueezeOpMaker, paddle::framework::DefaultGradOpMaker, paddle::framework::DefaultGradOpMaker); -REGISTER_OPERATOR(squeeze_grad, ops::SqueezeGradOp); +REGISTER_OPERATOR(squeeze_grad, ops::SqueezeGradOp, + ops::SqueezeGradNoNeedBufferVarsInference); REGISTER_OPERATOR(squeeze2, ops::Squeeze2Op, ops::Squeeze2OpMaker, ops::Squeeze2GradOpMaker, diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index a1614bfc8e782b..9669d240674316 100644 --- a/tools/check_api_approvals.sh +++ b/tools/check_api_approvals.sh @@ -26,7 +26,7 @@ API_FILES=("CMakeLists.txt" "python/paddle/fluid/framework.py" "python/paddle/fluid/backward.py" "paddle/fluid/operators/distributed/send_recv.proto.in" - "tools/generate_op_use_unused_inputs_white_list.py") + "paddle/fluid/framework/unused_var_check.h") approval_line=`curl -H "Authorization: token ${GITHUB_API_TOKEN}" https://api.github.com/repos/PaddlePaddle/Paddle/pulls/${GIT_PR_ID}/reviews?per_page=10000` git_files=`git diff --numstat upstream/$BRANCH| wc -l` @@ -79,7 +79,7 @@ for API_FILE in ${API_FILES[*]}; do APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 6836917 22361972` elif [ "${API_FILE}" == "paddle/fluid/operators/distributed/send_recv.proto.in" ];then APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 10721757 5442383` - elif [ "${API_FILE}" == "tools/generate_op_use_unused_inputs_white_list.py" ];then + elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.h" ];then APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 6888866 32832641 6836917` else APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 3048612 46782768 12538138 6836917 32832641` @@ -106,9 +106,9 @@ for API_FILE in ${API_FILES[*]}; do failed_num=`expr $failed_num + 1` echo_line="You must have one RD (gongweibao or seiriosPlus) approval for the paddle/fluid/operators/distributed/send_recv.proto.in, which manages the environment variables.\n" echo_list=(${echo_list[@]}$failed_num "." $echo_line) - elif [ "${API_FILE}" == "tools/generate_op_use_unused_inputs_white_list.py" ];then + elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.h" ];then failed_num=`expr $failed_num + 1` - echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the tools/generate_op_use_unused_inputs_white_list.py, which manages the white list of oerators that have unused input variables.\n" + echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the paddle/fluid/framework/unused_var_check.h, which manages the white list of oerators that have unused input variables.\n" echo_list=(${echo_list[@]}$failed_num "." $echo_line) else failed_num=`expr $failed_num + 1` diff --git a/tools/generate_op_use_unused_inputs_white_list.py b/tools/generate_op_use_unused_inputs_white_list.py deleted file mode 100644 index 79ed8b944934ce..00000000000000 --- a/tools/generate_op_use_unused_inputs_white_list.py +++ /dev/null @@ -1,42 +0,0 @@ -# Copyright (c) 2019 PaddlePaddle Authors. All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import os - -os.environ['CUDA_VISIBLE_DEVICES'] = '' - -import paddle.fluid as fluid -import sys - -white_list = [ - 'fill_zeros_like', - 'batch_norm', - 'crop', - 'cvm', - 'op_with_kernel', - 'dgc_momentum', - 'fake_quantize_range_abs_max', - 'flatten_grad', - 'batch_norm_grad', - 'squeeze_grad', -] - -if len(sys.argv) != 2: - print( - 'Usage: python generate_op_use_unused_inputs_white_list.py [filepath]') - sys.exit(1) - -with open(sys.argv[1], 'w') as f: - for op in white_list: - f.write(op + '\n') From 1db659b3a325ba2b5dd893226562950b7e6bbc2b Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Fri, 22 Nov 2019 11:16:07 +0800 Subject: [PATCH 07/17] solve failure of CI, test=develop --- paddle/fluid/framework/unused_var_check.h | 5 +++++ paddle/fluid/operators/center_loss_op.cc | 1 + paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc | 3 +++ paddle/fluid/operators/hierarchical_sigmoid_op.cc | 1 + paddle/fluid/operators/metrics/auc_op.cc | 4 ++-- paddle/fluid/operators/nce_op.cc | 2 ++ 6 files changed, 14 insertions(+), 2 deletions(-) diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h index 434d06ccc4298c..30430013e24483 100644 --- a/paddle/fluid/framework/unused_var_check.h +++ b/paddle/fluid/framework/unused_var_check.h @@ -27,16 +27,21 @@ namespace paddle { namespace framework { const std::unordered_set op_has_unsed_vars_white_list = { + "auc", "batch_norm", "batch_norm_grad", + "center_loss_grad", "crop", "cvm", "dgc_momentum", "fake_quantize_range_abs_max", "fill_zeros_like", + "fusion_seqpool_cvm_concat", "reshape2_grad_grad", "reshape2_grad", "gru_grad", + "hierarchical_sigmoid_grad", + "nce", "op_with_kernel"}; std::unordered_set* GetThreadLocalUsedVarNameSet(); diff --git a/paddle/fluid/operators/center_loss_op.cc b/paddle/fluid/operators/center_loss_op.cc index 02f1b349f1fb49..f0c0a5e619f7c9 100644 --- a/paddle/fluid/operators/center_loss_op.cc +++ b/paddle/fluid/operators/center_loss_op.cc @@ -134,6 +134,7 @@ class CenterLossOpGradMaker : public framework::SingleGradOpMaker { retv->SetType("center_loss_grad"); retv->SetInput(framework::GradVarName("Loss"), this->OutputGrad("Loss")); retv->SetInput("SampleCenterDiff", this->Output("SampleCenterDiff")); + retv->SetInput("X", this->Input("X")); retv->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); retv->SetAttrMap(this->Attrs()); diff --git a/paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc b/paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc index 633620278225b5..f64e4f134d62f1 100644 --- a/paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc +++ b/paddle/fluid/operators/fused/fusion_seqpool_cvm_concat_op.cc @@ -57,6 +57,9 @@ framework::OpKernelType FusionSeqPoolCVMConcatOp::GetExpectedKernelType( void FusionSeqPoolCVMConcatOpMaker::Make() { AddInput("X", "(LoDTensor) Input tensors of this operator.").AsDuplicable(); + AddInput("CVM", + "(Tensor), a 2-D Tensor with shape [N x 2], where N is the batch " + "size, 2 is show and click."); AddOutput("Out", "(LoDTensor) Output tensor of concat operator."); AddAttr("pooltype", "(string, default 'SUM') some of the pooling " diff --git a/paddle/fluid/operators/hierarchical_sigmoid_op.cc b/paddle/fluid/operators/hierarchical_sigmoid_op.cc index e76c833bbcd0b7..58e380183f1df6 100644 --- a/paddle/fluid/operators/hierarchical_sigmoid_op.cc +++ b/paddle/fluid/operators/hierarchical_sigmoid_op.cc @@ -195,6 +195,7 @@ class HierarchicalSigmoidGradMaker : public framework::SingleGradOpMaker { // Inputs: X, W, Label, PathTable, PathCode, PreOut, Out@GRAD op->SetInput("X", this->Input("X")); op->SetInput("W", this->Input("W")); + op->SetInput("Bias", this->Input("Bias")); op->SetInput("Label", this->Input("Label")); op->SetInput("PathTable", this->Input("PathTable")); op->SetInput("PathCode", this->Input("PathCode")); diff --git a/paddle/fluid/operators/metrics/auc_op.cc b/paddle/fluid/operators/metrics/auc_op.cc index 9d2845da08da3f..90734307accc6c 100644 --- a/paddle/fluid/operators/metrics/auc_op.cc +++ b/paddle/fluid/operators/metrics/auc_op.cc @@ -75,8 +75,8 @@ class AucOpMaker : public framework::OpProtoAndCheckerMaker { "shape: [batch_size, 1]"); // TODO(typhoonzero): support weight input - // AddInput("StatPos", "Statistic value when label = 1"); - // AddInput("StatNeg", "Statistic value when label = 0"); + AddInput("StatPos", "Statistic value when label = 1"); + AddInput("StatNeg", "Statistic value when label = 0"); AddOutput("AUC", "A scalar representing the " diff --git a/paddle/fluid/operators/nce_op.cc b/paddle/fluid/operators/nce_op.cc index ec600243a2f411..b6f68e3bee7da0 100644 --- a/paddle/fluid/operators/nce_op.cc +++ b/paddle/fluid/operators/nce_op.cc @@ -222,7 +222,9 @@ class NCEGradOpMaker : public framework::SingleGradOpMaker { op->SetType(this->ForwardOpType() + "_grad"); op->SetInput("Input", this->Input("Input")); op->SetInput("Label", this->Input("Label")); + op->SetInput("Bias", this->Input("Bias")); op->SetInput("Weight", this->Input("Weight")); + op->SetInput("Cost", this->Output("Cost")); op->SetInput("SampleLogits", this->Output("SampleLogits")); op->SetInput("SampleLabels", this->Output("SampleLabels")); op->SetInput("SampleWeight", this->Input("SampleWeight")); From 5b1047d4fe8c2de9b3e8f204843047144b3a2713 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Fri, 22 Nov 2019 14:55:32 +0800 Subject: [PATCH 08/17] add unittest for unused_var_check, test=develop --- paddle/fluid/framework/operator_test.cc | 109 +++++++++++++++++++++ paddle/fluid/framework/unused_var_check.cc | 20 ++-- paddle/fluid/framework/unused_var_check.h | 3 +- paddle/scripts/paddle_build.sh | 6 +- 4 files changed, 126 insertions(+), 12 deletions(-) diff --git a/paddle/fluid/framework/operator_test.cc b/paddle/fluid/framework/operator_test.cc index 77db37197095d7..bd2cd9147fad91 100644 --- a/paddle/fluid/framework/operator_test.cc +++ b/paddle/fluid/framework/operator_test.cc @@ -139,6 +139,8 @@ class CPUKernelTest : public OpKernel { cpu_kernel_run_num++; ASSERT_EQ(ctx.op().Input("x"), "IN1"); ASSERT_EQ(ctx.op().Output("y"), "OUT1"); + auto* x = ctx.Input("X"); + ASSERT_EQ(x, nullptr); } }; @@ -591,3 +593,110 @@ void SetGetLoDLevelTestMain(std::string op_type) { TEST(GetLoDLevelTest, base) { SetGetLoDLevelTestMain("get_lod_level_test"); } TEST(SetLoDLevelTest, base) { SetGetLoDLevelTestMain("set_lod_level_test"); } + +namespace paddle { +namespace framework { + +class OpUnusedVarTest : public OperatorWithKernel { + public: + using OperatorWithKernel::OperatorWithKernel; + + protected: + void InferShape(framework::InferShapeContext* ctx) const override {} + OpKernelType GetExpectedKernelType( + const ExecutionContext& ctx) const override { + return OpKernelType(proto::VarType::FP32, ctx.GetPlace(), + framework::DataLayout::kAnyLayout); + } +}; + +class OpUnusedVarTestProtoAndCheckerMaker : public OpProtoAndCheckerMaker { + public: + void Make() { + AddInput("X", "input of test op"); + AddOutput("Y", "output of test op"); + AddComment("This is test op for unused var check."); + } +}; + +template +class OpWithUnusedVarKernelTest : public OpKernel { + public: + void Compute(const ExecutionContext& ctx) const { + ASSERT_EQ(ctx.op().Input("X"), "X"); + ASSERT_EQ(ctx.op().Output("Y"), "Y"); + } +}; + +template +class OpWithoutUnusedVarKernelTest : public OpKernel { + public: + void Compute(const ExecutionContext& ctx) const { + ASSERT_EQ(ctx.op().Input("X"), "X"); + ASSERT_EQ(ctx.op().Output("Y"), "Y"); + auto* x = ctx.Input("X"); + auto* y = ctx.Output("Y"); + ASSERT_NE(x, y); + ASSERT_NE(y, nullptr); + } +}; + +} // namespace framework +} // namespace paddle + +REGISTER_OP_WITHOUT_GRADIENT( + op_with_unused_var, paddle::framework::OpUnusedVarTest, + paddle::framework::OpUnusedVarTestProtoAndCheckerMaker); + +REGISTER_OP_CPU_KERNEL(op_with_unused_var, + paddle::framework::OpWithUnusedVarKernelTest); + +REGISTER_OP_WITHOUT_GRADIENT( + op_without_unused_var, paddle::framework::OpUnusedVarTest, + paddle::framework::OpUnusedVarTestProtoAndCheckerMaker); + +REGISTER_OP_CPU_KERNEL(op_without_unused_var, + paddle::framework::OpWithoutUnusedVarKernelTest); + +// test with single input +TEST(OpWithUnusedVar, all) { + paddle::framework::InitDevices(true); + paddle::framework::proto::OpDesc op_desc; + op_desc.set_type("op_with_unused_var"); + BuildVar("X", {"X"}, op_desc.add_inputs()); + BuildVar("Y", {"Y"}, op_desc.add_outputs()); + + paddle::platform::CPUPlace cpu_place; + paddle::framework::Scope scope; + auto* x = scope.Var("X")->GetMutable(); + auto* y = scope.Var("Y")->GetMutable(); + x->Resize({32, 64}); + y->Resize({32, 64}); + x->mutable_data(cpu_place); + y->mutable_data(cpu_place); + + auto op = paddle::framework::OpRegistry::CreateOp(op_desc); + // should throw exception + ASSERT_THROW(op->Run(scope, cpu_place), paddle::platform::EnforceNotMet); +} + +TEST(OpWithoutUnusedVar, all) { + paddle::framework::InitDevices(true); + paddle::framework::proto::OpDesc op_desc; + op_desc.set_type("op_without_unused_var"); + BuildVar("X", {"X"}, op_desc.add_inputs()); + BuildVar("Y", {"Y"}, op_desc.add_outputs()); + + paddle::platform::CPUPlace cpu_place; + paddle::framework::Scope scope; + auto* x = scope.Var("X")->GetMutable(); + auto* y = scope.Var("Y")->GetMutable(); + x->Resize({32, 64}); + y->Resize({32, 64}); + x->mutable_data(cpu_place); + y->mutable_data(cpu_place); + + auto op = paddle::framework::OpRegistry::CreateOp(op_desc); + // should not throw exception + ASSERT_NO_THROW(op->Run(scope, cpu_place)); +} diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index c207be1708aa74..19e66a17cbe141 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -56,10 +56,12 @@ void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { if (used_set->count(pair.first) == 0) { for (auto &in_var_name : pair.second) { auto *in_var = scope.FindVar(in_var_name); - auto &tensor = in_var->Get(); - if (in_var->IsInitialized() && tensor.IsInitialized()) { - unsed_input_var_names.emplace_back(pair.first); - break; + if (in_var != nullptr && in_var->IsInitialized()) { + auto *tensor = &in_var->Get(); + if (tensor != nullptr && tensor->IsInitialized()) { + unsed_input_var_names.emplace_back(pair.first); + break; + } } } } @@ -70,8 +72,14 @@ void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { err_msg += in_var_name; err_msg += ", "; } - err_msg += "please remove it from inputs or register NoNeedBufferVars!"; - VLOG(1) << err_msg; + err_msg += + "please make sure it(them) is(are) needed. If not, remove it(them) " + "from inputs; if yes, register NoNeedBufferVars or add the operator to " + "white list in unused_var_check.h."; + // VLOG(1) << err_msg; + PADDLE_ENFORCE_EQ(unsed_input_var_names.size(), 0, + platform::errors::PermissionDenied( + "Unused input variables check failed: %s", err_msg)); } } diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h index 30430013e24483..91961d5b85ce68 100644 --- a/paddle/fluid/framework/unused_var_check.h +++ b/paddle/fluid/framework/unused_var_check.h @@ -41,8 +41,7 @@ const std::unordered_set op_has_unsed_vars_white_list = { "reshape2_grad", "gru_grad", "hierarchical_sigmoid_grad", - "nce", - "op_with_kernel"}; + "nce_grad"}; std::unordered_set* GetThreadLocalUsedVarNameSet(); void CheckUnusedVar(const OperatorBase& op, const Scope& scope); diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index bafb94661d2ed3..13816f1b45e6b3 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -436,11 +436,9 @@ EOF fi # TODO: jiabin need to refine this part when these tests fixed on mac - # ctest --output-on-failure -j $2 - # NOTE(cql): output logs of LOG(ERROR) to check unused vars. + # NOTE(cql): enable unused_var_check for MAC CI export FLAGS_enable_unused_var_check=1 - export GLOG_vmodule=unused_var_check=2 - ctest -VV -j $2 -O log.txt + ctest --output-on-failure -j $2 paddle version fi } From e97a64d7f3d4511774f1471b746c7fb12ea620e0 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Fri, 22 Nov 2019 19:53:39 +0800 Subject: [PATCH 09/17] refine code, enable check in operator_test, test=develop --- paddle/fluid/framework/operator.cc | 17 ++++++---------- paddle/fluid/framework/operator.h | 17 +++++----------- paddle/fluid/framework/operator_test.cc | 9 +++++++++ paddle/fluid/framework/unused_var_check.cc | 5 +++++ paddle/fluid/framework/unused_var_check.h | 7 ++++++- .../detection/roi_perspective_transform_op.cc | 2 ++ paddle/fluid/operators/prelu_op.cu | 20 +++++++++---------- paddle/scripts/paddle_build.sh | 4 ++-- 8 files changed, 44 insertions(+), 37 deletions(-) diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index 15ac2ec854fc1c..8f7e63f1e7a3c4 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -429,10 +429,8 @@ bool ExecutionContext::HasOutput(const std::string& name) const { } const Variable* ExecutionContext::InputVar(const std::string& name) const { - if (FLAGS_enable_unused_var_check) { - VLOG(6) << "InputVar: " << name; - GetThreadLocalUsedVarNameSet()->insert(name); - } + LogVarUsageIfUnusedVarCheckEnabled(name); + auto it = ctx_.inputs.find(name); if (it == ctx_.inputs.end()) return nullptr; @@ -462,10 +460,8 @@ const Tensor* ExecutionContext::Input(const std::string& name) const { template <> const std::vector ExecutionContext::MultiInput( const std::string& name) const { - if (FLAGS_enable_unused_var_check) { - VLOG(6) << "MultiInput: " << name; - GetThreadLocalUsedVarNameSet()->insert(name); - } + LogVarUsageIfUnusedVarCheckEnabled(name); + auto it = ctx_.inputs.find(name); if (it == ctx_.inputs.end()) { return {}; @@ -867,12 +863,11 @@ std::vector* OperatorWithKernel::GetKernelConfig( void OperatorWithKernel::RunImpl(const Scope& scope, const platform::Place& place) const { - // To reduce the elapsed time of HasAttr, we use bool variable to record the - // result of HasAttr. if (FLAGS_enable_unused_var_check) { GetThreadLocalUsedVarNameSet()->clear(); } - + // To reduce the elapsed time of HasAttr, we use bool variable to record the + // result of HasAttr. if (!enable_cache_runtime_context_ && HasAttr(kEnableCacheRuntimeContext)) enable_cache_runtime_context_ = true; if (!all_kernels_must_compute_runtime_shape_ && diff --git a/paddle/fluid/framework/operator.h b/paddle/fluid/framework/operator.h index ba5172fe390fea..9f635a3924db65 100644 --- a/paddle/fluid/framework/operator.h +++ b/paddle/fluid/framework/operator.h @@ -269,10 +269,8 @@ class ExecutionContext { const std::vector MultiInputVar( const std::string& name) const { - if (FLAGS_enable_unused_var_check) { - VLOG(6) << "MultiInputVar: " << name; - GetThreadLocalUsedVarNameSet()->insert(name); - } + LogVarUsageIfUnusedVarCheckEnabled(name); + auto it = ctx_.inputs.find(name); if (it == ctx_.inputs.end()) { return {}; @@ -303,10 +301,8 @@ class ExecutionContext { template const std::vector MultiInput(const std::string& name) const { - if (FLAGS_enable_unused_var_check) { - VLOG(6) << "MultiInput: " << name; - GetThreadLocalUsedVarNameSet()->insert(name); - } + LogVarUsageIfUnusedVarCheckEnabled(name); + auto it = ctx_.inputs.find(name); if (it == ctx_.inputs.end()) { return {}; @@ -358,10 +354,7 @@ class ExecutionContext { //! Get actual name vector for this input. const std::vector& Inputs(const std::string& name) const { - if (FLAGS_enable_unused_var_check) { - VLOG(6) << "Inputs: " << name; - GetThreadLocalUsedVarNameSet()->insert(name); - } + LogVarUsageIfUnusedVarCheckEnabled(name); return op_.Inputs(name); } diff --git a/paddle/fluid/framework/operator_test.cc b/paddle/fluid/framework/operator_test.cc index bd2cd9147fad91..389a9948be7797 100644 --- a/paddle/fluid/framework/operator_test.cc +++ b/paddle/fluid/framework/operator_test.cc @@ -18,6 +18,8 @@ limitations under the License. */ #include "paddle/fluid/framework/operator.h" #include "paddle/fluid/platform/init.h" +DECLARE_bool(enable_unused_var_check); + namespace paddle { namespace framework { @@ -660,6 +662,8 @@ REGISTER_OP_CPU_KERNEL(op_without_unused_var, // test with single input TEST(OpWithUnusedVar, all) { + // enable the unused_var_check + FLAGS_enable_unused_var_check = true; paddle::framework::InitDevices(true); paddle::framework::proto::OpDesc op_desc; op_desc.set_type("op_with_unused_var"); @@ -678,9 +682,13 @@ TEST(OpWithUnusedVar, all) { auto op = paddle::framework::OpRegistry::CreateOp(op_desc); // should throw exception ASSERT_THROW(op->Run(scope, cpu_place), paddle::platform::EnforceNotMet); + FLAGS_enable_unused_var_check = false; } TEST(OpWithoutUnusedVar, all) { + // enable the unused_var_check + FLAGS_enable_unused_var_check = true; + paddle::framework::InitDevices(true); paddle::framework::proto::OpDesc op_desc; op_desc.set_type("op_without_unused_var"); @@ -699,4 +707,5 @@ TEST(OpWithoutUnusedVar, all) { auto op = paddle::framework::OpRegistry::CreateOp(op_desc); // should not throw exception ASSERT_NO_THROW(op->Run(scope, cpu_place)); + FLAGS_enable_unused_var_check = false; } diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index 19e66a17cbe141..7cd7519e659139 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -34,6 +34,11 @@ std::unordered_set *GetThreadLocalUsedVarNameSet() { return &used_var_name_set; } +void LogVarUsageIfUnusedVarCheckEnabled(const std::string &name) { + VLOG(6) << "Variable used:" << name; + GetThreadLocalUsedVarNameSet()->insert(name); +} + void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { // skip op in white list if (op_has_unsed_vars_white_list.count(op.Type()) != 0) { diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h index 91961d5b85ce68..57cb357470f370 100644 --- a/paddle/fluid/framework/unused_var_check.h +++ b/paddle/fluid/framework/unused_var_check.h @@ -33,6 +33,7 @@ const std::unordered_set op_has_unsed_vars_white_list = { "center_loss_grad", "crop", "cvm", + "cos_sim_grad", "dgc_momentum", "fake_quantize_range_abs_max", "fill_zeros_like", @@ -41,9 +42,13 @@ const std::unordered_set op_has_unsed_vars_white_list = { "reshape2_grad", "gru_grad", "hierarchical_sigmoid_grad", - "nce_grad"}; + "nce_grad", + "roi_perspective_transform_grad"}; std::unordered_set* GetThreadLocalUsedVarNameSet(); +void InitThreadLocalUsedVarNameSet(); + +void LogVarUsageIfUnusedVarCheckEnabled(const std::string& name); void CheckUnusedVar(const OperatorBase& op, const Scope& scope); } // namespace framework diff --git a/paddle/fluid/operators/detection/roi_perspective_transform_op.cc b/paddle/fluid/operators/detection/roi_perspective_transform_op.cc index 6440cabf3769f0..b220ea540a899e 100644 --- a/paddle/fluid/operators/detection/roi_perspective_transform_op.cc +++ b/paddle/fluid/operators/detection/roi_perspective_transform_op.cc @@ -632,6 +632,8 @@ class ROIPerspectiveTransformGradMaker op->SetType("roi_perspective_transform_grad"); op->SetInput("X", this->Input("X")); op->SetInput("ROIs", this->Input("ROIs")); + op->SetInput("Out2InIdx", this->Output("Out2InIdx")); + op->SetInput("Out2InWeights", this->Output("Out2InWeights")); op->SetInput(framework::GradVarName("Out"), this->OutputGrad("Out")); op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); op->SetAttrMap(this->Attrs()); diff --git a/paddle/fluid/operators/prelu_op.cu b/paddle/fluid/operators/prelu_op.cu index 6d721e797ed39d..7e46a234834048 100644 --- a/paddle/fluid/operators/prelu_op.cu +++ b/paddle/fluid/operators/prelu_op.cu @@ -70,11 +70,11 @@ class CUDAPReluKernel : public framework::OpKernel { enum PRELU_MODE { Element, Channel, Scalar }; template -__global__ void PReluOpGradKernel(const T* x_ptr, const T* y_ptr, - const T* alpha_ptr, const T* dy_ptr, - T* dx_ptr, T* dalpha_ptr, size_t channel_num, - size_t plane_size, size_t spatial_size, - size_t numel, PRELU_MODE mode) { +__global__ void PReluOpGradKernel(const T* x_ptr, const T* alpha_ptr, + const T* dy_ptr, T* dx_ptr, T* dalpha_ptr, + size_t channel_num, size_t plane_size, + size_t spatial_size, size_t numel, + PRELU_MODE mode) { size_t index; CUDA_KERNEL_LOOP(index, numel) { T scale; @@ -98,15 +98,15 @@ __global__ void PReluOpGradKernel(const T* x_ptr, const T* y_ptr, template class PreluOpGradFunctor { public: - void operator()(cudaStream_t stream, const T* x, const T* y, const T* alpha, - const T* dy, T* dx, T* dalpha, std::vector input_shape, + void operator()(cudaStream_t stream, const T* x, const T* alpha, const T* dy, + T* dx, T* dalpha, std::vector input_shape, PRELU_MODE mode) { size_t plane_size = input_shape[2] * input_shape[3]; size_t spatial_size = plane_size * input_shape[1]; size_t numel = spatial_size * input_shape[0]; PReluOpGradKernel< T><<>>( - x, y, alpha, dy, dx, dalpha, input_shape[1], plane_size, spatial_size, + x, alpha, dy, dx, dalpha, input_shape[1], plane_size, spatial_size, numel, mode); } }; @@ -121,14 +121,12 @@ class CUDAPReluGradKernel : public framework::OpKernel { public: void Compute(const framework::ExecutionContext& context) const override { auto* x = context.Input("X"); - auto* y = context.Input("Out"); auto* alpha = context.Input("Alpha"); auto* dx = context.Output(framework::GradVarName("X")); auto* dy = context.Input(framework::GradVarName("Out")); auto* dalpha = context.Output(framework::GradVarName("Alpha")); const T* x_ptr = x->data(); - const T* y_ptr = y->data(); const T* alpha_ptr = alpha->data(); const T* dy_ptr = dy->data(); T* dx_ptr = dx ? dx->mutable_data(context.GetPlace()) : nullptr; @@ -163,7 +161,7 @@ class CUDAPReluGradKernel : public framework::OpKernel { m = Scalar; } PreluOpGradFunctor prelu_grad; - prelu_grad(stream, x_ptr, y_ptr, alpha_ptr, dy_ptr, dx_ptr, dalpha_tmp_ptr, + prelu_grad(stream, x_ptr, alpha_ptr, dy_ptr, dx_ptr, dalpha_tmp_ptr, input_shape, m); if (dalpha_tmp_ptr == nullptr) return; diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index 13816f1b45e6b3..0bd19c24a5961c 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -435,8 +435,8 @@ EOF pip3.7 install --user ${INSTALL_PREFIX:-/paddle/build}/opt/paddle/share/wheels/*.whl fi - # TODO: jiabin need to refine this part when these tests fixed on mac - # NOTE(cql): enable unused_var_check for MAC CI + # NOTE(zhiqiu): Set FLAGS_enable_unused_var_check=1 here to enable unused_var_check, + # which checks if an operator has unused input variable(s). export FLAGS_enable_unused_var_check=1 ctest --output-on-failure -j $2 paddle version From d45ae4026438e72d05af8ee3a76ce124d1b0c9d5 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Mon, 25 Nov 2019 20:55:05 +0800 Subject: [PATCH 10/17] skip mkldnn, test=develop --- paddle/fluid/framework/operator.cc | 18 ++++++++++------- paddle/fluid/framework/unused_var_check.cc | 23 ++++++++++++++++++++-- paddle/fluid/framework/unused_var_check.h | 20 +------------------ tools/check_api_approvals.sh | 8 ++++---- 4 files changed, 37 insertions(+), 32 deletions(-) diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index 8f7e63f1e7a3c4..aa64123e921bff 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -863,9 +863,6 @@ std::vector* OperatorWithKernel::GetKernelConfig( void OperatorWithKernel::RunImpl(const Scope& scope, const platform::Place& place) const { - if (FLAGS_enable_unused_var_check) { - GetThreadLocalUsedVarNameSet()->clear(); - } // To reduce the elapsed time of HasAttr, we use bool variable to record the // result of HasAttr. if (!enable_cache_runtime_context_ && HasAttr(kEnableCacheRuntimeContext)) @@ -887,10 +884,6 @@ void OperatorWithKernel::RunImpl(const Scope& scope, } RunImpl(scope, place, runtime_ctx_.get()); } - - if (FLAGS_enable_unused_var_check) { - CheckUnusedVar(*this, scope); - } } void OperatorWithKernel::RunImpl(const Scope& scope, @@ -922,6 +915,11 @@ void OperatorWithKernel::RunImpl(const Scope& scope, RuntimeInferShapeContext infer_shape_ctx(*this, exec_scope, *runtime_ctx); this->InferShape(&infer_shape_ctx); } + + if (FLAGS_enable_unused_var_check) { + GetThreadLocalUsedVarNameSet()->clear(); + } + // TODO(panyx0718): ExecutionContext should only depend on RuntimeContext // not Scope. Imperative mode only pass inputs and get outputs. (*kernel_func_)(ExecutionContext(*this, exec_scope, *dev_ctx, *runtime_ctx, @@ -931,6 +929,12 @@ void OperatorWithKernel::RunImpl(const Scope& scope, // there is inplace variable has been transfered. TransferInplaceVarsBack(scope, transfered_inplace_vars, *transfer_scope); } + if (FLAGS_enable_unused_var_check) { + // skip op that uses mkldnn because it has different memory reuse strategy. + if (kernel_type_->library_type_ != LibraryType::kMKLDNN) { + CheckUnusedVar(*this, scope); + } + } /*For profiling/benchmark only*/ if (FLAGS_benchmark) { diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index 7cd7519e659139..8c9aca3a67876c 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -26,6 +26,25 @@ DEFINE_bool(enable_unused_var_check, true, "Checking whether operator contains unused inputs, " "especially for grad operator. It should be in unittest."); +const std::unordered_set op_has_unsed_vars_white_list = { + "auc", + "batch_norm", + "batch_norm_grad", + "center_loss_grad", + "crop", + "cvm", + "cos_sim_grad", + "dgc_momentum", + "fake_quantize_range_abs_max", + "fill_zeros_like", + "fusion_seqpool_cvm_concat", + "reshape2_grad_grad", + "reshape2_grad", + "gru_grad", + "hierarchical_sigmoid_grad", + "nce_grad", + "roi_perspective_transform_grad"}; + namespace paddle { namespace framework { @@ -40,7 +59,7 @@ void LogVarUsageIfUnusedVarCheckEnabled(const std::string &name) { } void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { - // skip op in white list + // skip op in white list and it should be fixed in the future. if (op_has_unsed_vars_white_list.count(op.Type()) != 0) { return; } @@ -80,7 +99,7 @@ void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { err_msg += "please make sure it(them) is(are) needed. If not, remove it(them) " "from inputs; if yes, register NoNeedBufferVars or add the operator to " - "white list in unused_var_check.h."; + "white list in unused_var_check.cc."; // VLOG(1) << err_msg; PADDLE_ENFORCE_EQ(unsed_input_var_names.size(), 0, platform::errors::PermissionDenied( diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h index 57cb357470f370..f7b1cdefd87fd6 100644 --- a/paddle/fluid/framework/unused_var_check.h +++ b/paddle/fluid/framework/unused_var_check.h @@ -22,29 +22,11 @@ limitations under the License. */ #include "paddle/fluid/framework/operator.h" DECLARE_bool(enable_unused_var_check); +DECLARE_bool(use_mkldnn); namespace paddle { namespace framework { -const std::unordered_set op_has_unsed_vars_white_list = { - "auc", - "batch_norm", - "batch_norm_grad", - "center_loss_grad", - "crop", - "cvm", - "cos_sim_grad", - "dgc_momentum", - "fake_quantize_range_abs_max", - "fill_zeros_like", - "fusion_seqpool_cvm_concat", - "reshape2_grad_grad", - "reshape2_grad", - "gru_grad", - "hierarchical_sigmoid_grad", - "nce_grad", - "roi_perspective_transform_grad"}; - std::unordered_set* GetThreadLocalUsedVarNameSet(); void InitThreadLocalUsedVarNameSet(); diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index 9669d240674316..32876df476fdd2 100644 --- a/tools/check_api_approvals.sh +++ b/tools/check_api_approvals.sh @@ -26,7 +26,7 @@ API_FILES=("CMakeLists.txt" "python/paddle/fluid/framework.py" "python/paddle/fluid/backward.py" "paddle/fluid/operators/distributed/send_recv.proto.in" - "paddle/fluid/framework/unused_var_check.h") + "paddle/fluid/framework/unused_var_check.cc") approval_line=`curl -H "Authorization: token ${GITHUB_API_TOKEN}" https://api.github.com/repos/PaddlePaddle/Paddle/pulls/${GIT_PR_ID}/reviews?per_page=10000` git_files=`git diff --numstat upstream/$BRANCH| wc -l` @@ -79,7 +79,7 @@ for API_FILE in ${API_FILES[*]}; do APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 6836917 22361972` elif [ "${API_FILE}" == "paddle/fluid/operators/distributed/send_recv.proto.in" ];then APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 10721757 5442383` - elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.h" ];then + elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.cc" ];then APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 6888866 32832641 6836917` else APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 3048612 46782768 12538138 6836917 32832641` @@ -106,9 +106,9 @@ for API_FILE in ${API_FILES[*]}; do failed_num=`expr $failed_num + 1` echo_line="You must have one RD (gongweibao or seiriosPlus) approval for the paddle/fluid/operators/distributed/send_recv.proto.in, which manages the environment variables.\n" echo_list=(${echo_list[@]}$failed_num "." $echo_line) - elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.h" ];then + elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.cc" ];then failed_num=`expr $failed_num + 1` - echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the paddle/fluid/framework/unused_var_check.h, which manages the white list of oerators that have unused input variables.\n" + echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the paddle/fluid/framework/unused_var_check.cc, which manages the white list of oerators that have unused input variables.\n" echo_list=(${echo_list[@]}$failed_num "." $echo_line) else failed_num=`expr $failed_num + 1` From 7f2b8ca7a6afe3fbc18ed403d8901ad27f2bfc26 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Tue, 26 Nov 2019 01:04:34 +0800 Subject: [PATCH 11/17] extend white list, test=develop --- paddle/fluid/framework/unused_var_check.cc | 16 +++++++++++++++- paddle/fluid/operators/batch_norm_op.cc | 1 + 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index 8c9aca3a67876c..811ab3a116f7d7 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -30,6 +30,7 @@ const std::unordered_set op_has_unsed_vars_white_list = { "auc", "batch_norm", "batch_norm_grad", + "sync_batch_norm_grad", "center_loss_grad", "crop", "cvm", @@ -43,7 +44,20 @@ const std::unordered_set op_has_unsed_vars_white_list = { "gru_grad", "hierarchical_sigmoid_grad", "nce_grad", - "roi_perspective_transform_grad"}; + "roi_perspective_transform_grad", + "sequence_conv_grad", + "gru_unit_grad", + "affine_grid_grad", + "fill_any_like", + "precision_recall", + "unsqueeze_grad", + "kldiv_loss_grad", + "cvm_grad", + "stack_grad", + "warpctc_grad", + "sync_batch_norm", + "match_matrix_tensor_grad", + "ngraph_engine"}; namespace paddle { namespace framework { diff --git a/paddle/fluid/operators/batch_norm_op.cc b/paddle/fluid/operators/batch_norm_op.cc index fb43832d7f61b6..ab72766b90c679 100644 --- a/paddle/fluid/operators/batch_norm_op.cc +++ b/paddle/fluid/operators/batch_norm_op.cc @@ -621,6 +621,7 @@ std::unique_ptr BatchNormGradMaker::Apply() const { op->SetInput(framework::GradVarName("Y"), this->OutputGrad("Y")); op->SetInput("Scale", this->Input("Scale")); + op->SetInput("Bias", this->Input("Bias")); op->SetInput("SavedMean", this->Output("SavedMean")); op->SetInput("SavedVariance", this->Output("SavedVariance")); From 67b9f4fd35a9a8543d1f5ffdeea0c9bd6f9ebad7 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Wed, 27 Nov 2019 12:34:58 +0800 Subject: [PATCH 12/17] refine condition of mkldnn, test=develop --- paddle/fluid/framework/operator.cc | 4 +++- paddle/scripts/paddle_build.sh | 11 ++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index aa64123e921bff..68e564b16d4d7e 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -931,7 +931,9 @@ void OperatorWithKernel::RunImpl(const Scope& scope, } if (FLAGS_enable_unused_var_check) { // skip op that uses mkldnn because it has different memory reuse strategy. - if (kernel_type_->library_type_ != LibraryType::kMKLDNN) { + // use attr here because some GradMakers (like ActivationGradOpMaker) add + // input when use_mkldnn=true; + if (!(HasAttr("use_mkldnn") && Attr("use_mkldnn"))) { CheckUnusedVar(*this, scope); } } diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index 0bd19c24a5961c..1a7be8d4beabcd 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -435,9 +435,6 @@ EOF pip3.7 install --user ${INSTALL_PREFIX:-/paddle/build}/opt/paddle/share/wheels/*.whl fi - # NOTE(zhiqiu): Set FLAGS_enable_unused_var_check=1 here to enable unused_var_check, - # which checks if an operator has unused input variable(s). - export FLAGS_enable_unused_var_check=1 ctest --output-on-failure -j $2 paddle version fi @@ -706,6 +703,13 @@ function parallel_test() { parallel_test_base } +function enable_unused_var_check() { + # NOTE(zhiqiu): Set FLAGS_enable_unused_var_check=1 here to enable unused_var_check, + # which checks if an operator has unused input variable(s). + # Currently, use it in coverage CI job. + export FLAGS_enable_unused_var_check=1 +} + function gen_doc_lib() { mkdir -p ${PADDLE_ROOT}/build cd ${PADDLE_ROOT}/build @@ -1076,6 +1080,7 @@ function main() { cicheck) cmake_gen ${PYTHON_ABI:-""} build ${parallel_number} + enable_unused_var_check() parallel_test ;; cicheck_brpc) From a1800790f661ef8c5b6ec5b8a7f08285046592a2 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Wed, 27 Nov 2019 13:28:54 +0800 Subject: [PATCH 13/17] fix paddle_build, test=develop --- paddle/fluid/framework/unused_var_check.cc | 2 +- paddle/scripts/paddle_build.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index 811ab3a116f7d7..2923c0bc9cc4bf 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -22,7 +22,7 @@ limitations under the License. */ #include "paddle/fluid/framework/unused_var_check.h" #include "paddle/fluid/platform/enforce.h" -DEFINE_bool(enable_unused_var_check, true, +DEFINE_bool(enable_unused_var_check, false, "Checking whether operator contains unused inputs, " "especially for grad operator. It should be in unittest."); diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index 1a7be8d4beabcd..e218812074cda4 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -1080,7 +1080,7 @@ function main() { cicheck) cmake_gen ${PYTHON_ABI:-""} build ${parallel_number} - enable_unused_var_check() + enable_unused_var_check parallel_test ;; cicheck_brpc) From a470fbc5910fb09ee4e6e434fb0e67b9b627bd41 Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Thu, 28 Nov 2019 11:49:52 +0800 Subject: [PATCH 14/17] follow comments, test=develop --- paddle/fluid/framework/unused_var_check.cc | 12 ++++++---- paddle/fluid/framework/unused_var_check.h | 1 - paddle/fluid/operators/flatten_op.cc | 27 +++++++++++++++++----- paddle/fluid/operators/squeeze_op.cc | 25 ++++++++++++++++---- 4 files changed, 48 insertions(+), 17 deletions(-) diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index 2923c0bc9cc4bf..1ea88f4c353072 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -68,8 +68,10 @@ std::unordered_set *GetThreadLocalUsedVarNameSet() { } void LogVarUsageIfUnusedVarCheckEnabled(const std::string &name) { - VLOG(6) << "Variable used:" << name; - GetThreadLocalUsedVarNameSet()->insert(name); + if (FLAGS_enable_unused_var_check) { + VLOG(6) << "Variable used:" << name; + GetThreadLocalUsedVarNameSet()->insert(name); + } } void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { @@ -111,10 +113,10 @@ void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { err_msg += ", "; } err_msg += - "please make sure it(them) is(are) needed. If not, remove it(them) " - "from inputs; if yes, register NoNeedBufferVars or add the operator to " + "please make sure it(they) is(are) needed. If not, remove it(them) " + "from inputs of the operator; if yes, register NoNeedBufferVars or add " + "the operator to " "white list in unused_var_check.cc."; - // VLOG(1) << err_msg; PADDLE_ENFORCE_EQ(unsed_input_var_names.size(), 0, platform::errors::PermissionDenied( "Unused input variables check failed: %s", err_msg)); diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h index f7b1cdefd87fd6..2c465dffb095ce 100644 --- a/paddle/fluid/framework/unused_var_check.h +++ b/paddle/fluid/framework/unused_var_check.h @@ -22,7 +22,6 @@ limitations under the License. */ #include "paddle/fluid/framework/operator.h" DECLARE_bool(enable_unused_var_check); -DECLARE_bool(use_mkldnn); namespace paddle { namespace framework { diff --git a/paddle/fluid/operators/flatten_op.cc b/paddle/fluid/operators/flatten_op.cc index 59509686de0d31..87ab2fb6f9bba9 100644 --- a/paddle/fluid/operators/flatten_op.cc +++ b/paddle/fluid/operators/flatten_op.cc @@ -137,6 +137,22 @@ class FlattenGradOp : public framework::OperatorWithKernel { } }; +template +class FlattenGradOpMaker : public framework::SingleGradOpMaker { + public: + using framework::SingleGradOpMaker::SingleGradOpMaker; + + std::unique_ptr Apply() const override { + auto *grad_op = new T(); + grad_op->SetType("flatten_grad"); + grad_op->SetInput("X", this->Input("X")); + grad_op->SetInput(framework::GradVarName("Out"), this->OutputGrad("Out")); + grad_op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); + grad_op->SetAttrMap(this->Attrs()); + return std::unique_ptr(grad_op); + } +}; + // FIXME(zcd): flatten2 adds an intermediate output(XShape) based on flatten, // the XShape is used to carry the shape and lod of X which will be used in // flatten_grad, in this way, the framework can reuse the memory of X @@ -235,17 +251,16 @@ DECLARE_INPLACE_OP_INFERER(FlattenGradInplaceinToOut, {framework::GradVarName("Out"), framework::GradVarName("X")}); DECLARE_NO_NEED_BUFFER_VARS_INFERENCE(FlattenGradNoNeedBufferVarsInference, - "Out"); + "X"); } // namespace operators } // namespace paddle namespace ops = paddle::operators; -REGISTER_OPERATOR( - flatten, ops::FlattenOp, ops::FlattenOpMaker, - paddle::framework::DefaultGradOpMaker, - paddle::framework::DefaultGradOpMaker, - ops::FlattenOpInplaceInToOut); +REGISTER_OPERATOR(flatten, ops::FlattenOp, ops::FlattenOpMaker, + ops::FlattenGradOpMaker, + ops::FlattenGradOpMaker, + ops::FlattenOpInplaceInToOut); REGISTER_OPERATOR(flatten_grad, ops::FlattenGradOp, ops::FlattenGradInplaceinToOut, ops::FlattenGradNoNeedBufferVarsInference); diff --git a/paddle/fluid/operators/squeeze_op.cc b/paddle/fluid/operators/squeeze_op.cc index 58b052680318d0..025cb69ae6d0b8 100644 --- a/paddle/fluid/operators/squeeze_op.cc +++ b/paddle/fluid/operators/squeeze_op.cc @@ -214,6 +214,22 @@ class Squeeze2Op : public framework::OperatorWithKernel { } }; +template +class SqueezeGradOpMaker : public framework::SingleGradOpMaker { + public: + using framework::SingleGradOpMaker::SingleGradOpMaker; + + std::unique_ptr Apply() const override { + auto *grad_op = new T(); + grad_op->SetType("squeeze_grad"); + grad_op->SetInput("X", this->Input("X")); + grad_op->SetInput(framework::GradVarName("Out"), this->OutputGrad("Out")); + grad_op->SetOutput(framework::GradVarName("X"), this->InputGrad("X")); + grad_op->SetAttrMap(this->Attrs()); + return std::unique_ptr(grad_op); + } +}; + class Squeeze2GradOp : public framework::OperatorWithKernel { public: using framework::OperatorWithKernel::OperatorWithKernel; @@ -275,15 +291,14 @@ DECLARE_INPLACE_OP_INFERER(SequeezeGradInplaceInferer, {framework::GradVarName("Out"), framework::GradVarName("X")}); DECLARE_NO_NEED_BUFFER_VARS_INFERENCE(SqueezeGradNoNeedBufferVarsInference, - "Out"); + "X"); } // namespace operators } // namespace paddle namespace ops = paddle::operators; -REGISTER_OPERATOR( - squeeze, ops::SqueezeOp, ops::SqueezeOpMaker, - paddle::framework::DefaultGradOpMaker, - paddle::framework::DefaultGradOpMaker); +REGISTER_OPERATOR(squeeze, ops::SqueezeOp, ops::SqueezeOpMaker, + ops::SqueezeGradOpMaker, + ops::SqueezeGradOpMaker); REGISTER_OPERATOR(squeeze_grad, ops::SqueezeGradOp, ops::SqueezeGradNoNeedBufferVarsInference); From 98f150998889a3dbacb2dd31f373ef40fdd7c5dc Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Thu, 28 Nov 2019 15:22:08 +0800 Subject: [PATCH 15/17] fix GetExpectedKernelType --- paddle/fluid/framework/operator.cc | 1 + paddle/fluid/framework/unused_var_check.h | 2 -- paddle/fluid/operators/flatten_op.cc | 6 +++--- paddle/fluid/operators/squeeze_op.cc | 6 +++--- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index 68e564b16d4d7e..73b6253395d8ba 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -34,6 +34,7 @@ limitations under the License. */ DECLARE_bool(benchmark); DECLARE_bool(check_nan_inf); +DECLARE_bool(enable_unused_var_check); DEFINE_int32(inner_op_parallelism, 0, "number of threads for inner op"); DEFINE_bool(fast_check_nan_inf, false, "Fast checking NAN/INF after each operation. It will be a little" diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h index 2c465dffb095ce..f25368d7e51981 100644 --- a/paddle/fluid/framework/unused_var_check.h +++ b/paddle/fluid/framework/unused_var_check.h @@ -21,8 +21,6 @@ limitations under the License. */ #include #include "paddle/fluid/framework/operator.h" -DECLARE_bool(enable_unused_var_check); - namespace paddle { namespace framework { diff --git a/paddle/fluid/operators/flatten_op.cc b/paddle/fluid/operators/flatten_op.cc index 87ab2fb6f9bba9..19a708cfbddc3f 100644 --- a/paddle/fluid/operators/flatten_op.cc +++ b/paddle/fluid/operators/flatten_op.cc @@ -131,9 +131,9 @@ class FlattenGradOp : public framework::OperatorWithKernel { protected: framework::OpKernelType GetExpectedKernelType( const framework::ExecutionContext &ctx) const override { - return framework::OpKernelType( - OperatorWithKernel::IndicateVarDataType(ctx, "X"), - ctx.device_context()); + return framework::OpKernelType(OperatorWithKernel::IndicateVarDataType( + ctx, framework::GradVarName("Out")), + ctx.device_context()); } }; diff --git a/paddle/fluid/operators/squeeze_op.cc b/paddle/fluid/operators/squeeze_op.cc index 025cb69ae6d0b8..9d394cd62ddb21 100644 --- a/paddle/fluid/operators/squeeze_op.cc +++ b/paddle/fluid/operators/squeeze_op.cc @@ -123,9 +123,9 @@ class SqueezeGradOp : public framework::OperatorWithKernel { protected: framework::OpKernelType GetExpectedKernelType( const framework::ExecutionContext &ctx) const override { - return framework::OpKernelType( - OperatorWithKernel::IndicateVarDataType(ctx, "X"), - ctx.device_context()); + return framework::OpKernelType(OperatorWithKernel::IndicateVarDataType( + ctx, framework::GradVarName("Out")), + ctx.device_context()); } }; From 6ee0ab02b5fe10a272d4816d1abb7b13fafdac3f Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Thu, 28 Nov 2019 15:41:55 +0800 Subject: [PATCH 16/17] add wiki ref to err_msg, test=develop --- paddle/fluid/framework/unused_var_check.cc | 7 +++++-- tools/check_api_approvals.sh | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/paddle/fluid/framework/unused_var_check.cc b/paddle/fluid/framework/unused_var_check.cc index 1ea88f4c353072..1d433bfbd00d0e 100644 --- a/paddle/fluid/framework/unused_var_check.cc +++ b/paddle/fluid/framework/unused_var_check.cc @@ -114,9 +114,12 @@ void CheckUnusedVar(const OperatorBase &op, const Scope &scope) { } err_msg += "please make sure it(they) is(are) needed. If not, remove it(them) " - "from inputs of the operator; if yes, register NoNeedBufferVars or add " + "from inputs of the operator; if yes, register " + "NoNeedBufferVarsInference or add " "the operator to " - "white list in unused_var_check.cc."; + "white list in unused_var_check.cc. See more details at " + "[https://github.com/PaddlePaddle/Paddle/wiki/" + "OP-Should-Not-Have-Unused-Input]"; PADDLE_ENFORCE_EQ(unsed_input_var_names.size(), 0, platform::errors::PermissionDenied( "Unused input variables check failed: %s", err_msg)); diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index 9a69e326b78403..97b1039019ca62 100644 --- a/tools/check_api_approvals.sh +++ b/tools/check_api_approvals.sh @@ -91,7 +91,7 @@ for API_FILE in ${API_FILES[*]}; do echo_line="You must have one RD (gongweibao or seiriosPlus) approval for the paddle/fluid/operators/distributed/send_recv.proto.in, which manages the environment variables.\n" check_approval 1 10721757 5442383 elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.cc" ];then - echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the paddle/fluid/framework/unused_var_check.cc, which manages the white list of operators that have unused input variables.\n" + echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the paddle/fluid/framework/unused_var_check.cc, which manages the white list of operators that have unused input variables. Before change the white list, please read the spicification [https://github.com/PaddlePaddle/Paddle/wiki/OP-Should-Not-Have-Unused-Input] and try to refine code first. \n" check_approval 1 6888866 32832641 6836917 else echo_line="You must have one RD (XiaoguangHu01,Xreki,luotao1,sneaxiy) approval for ${API_FILE}, which manages the underlying code for fluid.\n" From cfc3a6e8309b1b8f17c5707ad22125730fd9143b Mon Sep 17 00:00:00 2001 From: zhiqiu Date: Fri, 29 Nov 2019 12:06:54 +0800 Subject: [PATCH 17/17] follow comment, test=develop --- paddle/fluid/framework/unused_var_check.h | 1 - 1 file changed, 1 deletion(-) diff --git a/paddle/fluid/framework/unused_var_check.h b/paddle/fluid/framework/unused_var_check.h index f25368d7e51981..b56498e0e7f793 100644 --- a/paddle/fluid/framework/unused_var_check.h +++ b/paddle/fluid/framework/unused_var_check.h @@ -25,7 +25,6 @@ namespace paddle { namespace framework { std::unordered_set* GetThreadLocalUsedVarNameSet(); -void InitThreadLocalUsedVarNameSet(); void LogVarUsageIfUnusedVarCheckEnabled(const std::string& name); void CheckUnusedVar(const OperatorBase& op, const Scope& scope);