From 2a061aa5b4f4847cf1d5293c1591b69a50c15dd0 Mon Sep 17 00:00:00 2001 From: sunsuodong Date: Fri, 11 Dec 2020 16:24:12 +0800 Subject: [PATCH] fix code review --- mindspore/lite/schema/ops.fbs | 2 +- mindspore/lite/src/ops/populate/transpose_populate.cc | 1 - mindspore/lite/src/ops/transpose.cc | 9 --------- mindspore/lite/src/ops/transpose.h | 2 -- .../lite/src/runtime/kernel/arm/fp32_grad/adam.cc | 1 - .../src/runtime/kernel/arm/fp32_grad/apply_momentum.cc | 1 - .../lite/src/runtime/kernel/arm/fp32_grad/assign.cc | 1 - mindspore/lite/src/runtime/kernel/arm/fp32_grad/sgd.cc | 1 - .../parser/tflite/tflite_transpose_parser_test.cc | 1 - mindspore/lite/tools/benchmark/benchmark.cc | 2 +- mindspore/lite/tools/benchmark/benchmark.h | 2 +- .../fusion/matmul_biasadd_fusion_pass.cc | 1 - .../fusion/matmul_biasadd_fusion_pass.h | 10 +++++++--- .../legacy_optimizer/graph/format_trans_pass.h | 2 +- .../converter/parser/caffe/caffe_permute_parser.cc | 1 - .../converter/parser/onnx/onnx_transpose_parser.cc | 1 - .../tools/converter/parser/tf/tf_transpose_parser.cc | 1 - .../converter/parser/tflite/tflite_transpose_parser.cc | 1 - .../lite/tools/converter/parser/tflite/tflite_util.cc | 2 +- 19 files changed, 12 insertions(+), 30 deletions(-) diff --git a/mindspore/lite/schema/ops.fbs b/mindspore/lite/schema/ops.fbs index 9dea38d0af..4b292964f2 100644 --- a/mindspore/lite/schema/ops.fbs +++ b/mindspore/lite/schema/ops.fbs @@ -781,7 +781,7 @@ table Reduce { table Transpose { perm: [int]; - conjugate: bool = false; + conjugate: bool = false; // DEPRECATED } table Squeeze { diff --git a/mindspore/lite/src/ops/populate/transpose_populate.cc b/mindspore/lite/src/ops/populate/transpose_populate.cc index 509cf1dc46..ecd2686b01 100644 --- a/mindspore/lite/src/ops/populate/transpose_populate.cc +++ b/mindspore/lite/src/ops/populate/transpose_populate.cc @@ -39,7 +39,6 @@ OpParameter *PopulateTransposeParameter(const mindspore::lite::PrimitiveC *primi transpose_param->perm_[i++] = *iter; } transpose_param->num_axes_ = i; - transpose_param->conjugate_ = param->GetConjugate(); return reinterpret_cast(transpose_param); } diff --git a/mindspore/lite/src/ops/transpose.cc b/mindspore/lite/src/ops/transpose.cc index 50f4f876c1..63e2f111a4 100644 --- a/mindspore/lite/src/ops/transpose.cc +++ b/mindspore/lite/src/ops/transpose.cc @@ -27,10 +27,7 @@ namespace mindspore { namespace lite { #ifdef PRIMITIVE_WRITEABLE std::vector Transpose::GetPerm() const { return this->primitive_->value.AsTranspose()->perm; } -bool Transpose::GetConjugate() const { return this->primitive_->value.AsTranspose()->conjugate; } - void Transpose::SetPerm(const std::vector &perm) { this->primitive_->value.AsTranspose()->perm = perm; } -void Transpose::SetConjugate(bool conjugate) { this->primitive_->value.AsTranspose()->conjugate = conjugate; } int Transpose::UnPackAttr(const Primitive &prim, const std::vector &inputs) { if (this->primitive_ == nullptr) { @@ -83,7 +80,6 @@ std::vector Transpose::GetPerm() const { auto fb_vector = this->primitive_->value_as_Transpose()->perm(); return std::vector(fb_vector->begin(), fb_vector->end()); } -bool Transpose::GetConjugate() const { return this->primitive_->value_as_Transpose()->conjugate(); } int Transpose::UnPackToFlatBuilder(const schema::Primitive *primitive, flatbuffers::FlatBufferBuilder *fbb) { MS_ASSERT(nullptr != primitive); @@ -127,11 +123,6 @@ int Transpose::InferShape(std::vector inputs_, std::vector o MS_ASSERT(inputs_.size() == kSingleNum || inputs_.size() == kDoubleNum); MS_ASSERT(outputs_.size() == kSingleNum); - int conjugate = GetConjugate(); - if (conjugate) { - MS_LOG(ERROR) << "Transpose conjugate is not support currently"; - return RET_ERROR; - } std::vector perm; for (size_t i = 0; i < GetPerm().size(); i++) { perm.push_back(GetPerm().at(i)); diff --git a/mindspore/lite/src/ops/transpose.h b/mindspore/lite/src/ops/transpose.h index 6d907716a2..adb5be37a8 100644 --- a/mindspore/lite/src/ops/transpose.h +++ b/mindspore/lite/src/ops/transpose.h @@ -35,13 +35,11 @@ class Transpose : public PrimitiveC { explicit Transpose(schema::PrimitiveT *primitive) : PrimitiveC(primitive) {} int UnPackAttr(const Primitive &prim, const std::vector &inputs) override; void SetPerm(const std::vector &perm); - void SetConjugate(bool conjugate); #else int UnPackToFlatBuilder(const schema::Primitive *primitive, flatbuffers::FlatBufferBuilder *fbb) override; #endif int InferShape(std::vector inputs_, std::vector outputs_) override; std::vector GetPerm() const; - bool GetConjugate() const; }; } // namespace lite } // namespace mindspore diff --git a/mindspore/lite/src/runtime/kernel/arm/fp32_grad/adam.cc b/mindspore/lite/src/runtime/kernel/arm/fp32_grad/adam.cc index e040e0f337..9dac7deaa0 100644 --- a/mindspore/lite/src/runtime/kernel/arm/fp32_grad/adam.cc +++ b/mindspore/lite/src/runtime/kernel/arm/fp32_grad/adam.cc @@ -21,7 +21,6 @@ #include "src/kernel_registry.h" #include "include/errorcode.h" #include "src/runtime/runtime_api.h" -#include "src/runtime/kernel/arm/fp32/nchw2nhwc_fp32.h" using mindspore::kernel::KERNEL_ARCH::kCPU; using mindspore::lite::KernelRegistrar; diff --git a/mindspore/lite/src/runtime/kernel/arm/fp32_grad/apply_momentum.cc b/mindspore/lite/src/runtime/kernel/arm/fp32_grad/apply_momentum.cc index 5667275628..6213df7697 100644 --- a/mindspore/lite/src/runtime/kernel/arm/fp32_grad/apply_momentum.cc +++ b/mindspore/lite/src/runtime/kernel/arm/fp32_grad/apply_momentum.cc @@ -20,7 +20,6 @@ #include "src/kernel_registry.h" #include "include/errorcode.h" #include "src/runtime/runtime_api.h" -#include "src/runtime/kernel/arm/fp32/nchw2nhwc_fp32.h" using mindspore::kernel::KERNEL_ARCH::kCPU; using mindspore::lite::KernelRegistrar; diff --git a/mindspore/lite/src/runtime/kernel/arm/fp32_grad/assign.cc b/mindspore/lite/src/runtime/kernel/arm/fp32_grad/assign.cc index 792fb0e683..df4b6a686a 100644 --- a/mindspore/lite/src/runtime/kernel/arm/fp32_grad/assign.cc +++ b/mindspore/lite/src/runtime/kernel/arm/fp32_grad/assign.cc @@ -20,7 +20,6 @@ #include "src/kernel_registry.h" #include "include/errorcode.h" #include "src/runtime/runtime_api.h" -#include "src/runtime/kernel/arm/fp32/nchw2nhwc_fp32.h" using mindspore::kernel::KERNEL_ARCH::kCPU; using mindspore::lite::KernelRegistrar; diff --git a/mindspore/lite/src/runtime/kernel/arm/fp32_grad/sgd.cc b/mindspore/lite/src/runtime/kernel/arm/fp32_grad/sgd.cc index 9c1d6d9b47..731b766a50 100644 --- a/mindspore/lite/src/runtime/kernel/arm/fp32_grad/sgd.cc +++ b/mindspore/lite/src/runtime/kernel/arm/fp32_grad/sgd.cc @@ -20,7 +20,6 @@ #include "src/kernel_registry.h" #include "include/errorcode.h" #include "src/runtime/runtime_api.h" -#include "src/runtime/kernel/arm/fp32/nchw2nhwc_fp32.h" using mindspore::kernel::KERNEL_ARCH::kCPU; using mindspore::lite::KernelRegistrar; diff --git a/mindspore/lite/test/ut/tools/converter/parser/tflite/tflite_transpose_parser_test.cc b/mindspore/lite/test/ut/tools/converter/parser/tflite/tflite_transpose_parser_test.cc index d2af32eab7..28bb1ba51f 100644 --- a/mindspore/lite/test/ut/tools/converter/parser/tflite/tflite_transpose_parser_test.cc +++ b/mindspore/lite/test/ut/tools/converter/parser/tflite/tflite_transpose_parser_test.cc @@ -35,7 +35,6 @@ TEST_F(TestTfliteParserTranspose, OpType) { TEST_F(TestTfliteParserTranspose, AttrValue) { ASSERT_NE(meta_graph->nodes.front()->primitive->value.AsTranspose(), nullptr); auto val = meta_graph->nodes.front()->primitive->value.AsTranspose(); - ASSERT_EQ(val->conjugate, false); std::vector perm = {1, 0}; ASSERT_EQ(val->perm, perm); } diff --git a/mindspore/lite/tools/benchmark/benchmark.cc b/mindspore/lite/tools/benchmark/benchmark.cc index ac7c936a64..d382e850b4 100644 --- a/mindspore/lite/tools/benchmark/benchmark.cc +++ b/mindspore/lite/tools/benchmark/benchmark.cc @@ -203,7 +203,7 @@ int Benchmark::ReadTensorData(std::ifstream &in_file_stream, const std::string & } auto *check_tensor = new (std::nothrow) CheckTensor(dims, data, strings_data); if (check_tensor == nullptr) { - MS_LOG(ERROR) << "Now CheckTensor failed, tensor name: " << tensor_name; + MS_LOG(ERROR) << "New CheckTensor failed, tensor name: " << tensor_name; return RET_ERROR; } this->benchmark_data_.insert(std::make_pair(tensor_name, check_tensor)); diff --git a/mindspore/lite/tools/benchmark/benchmark.h b/mindspore/lite/tools/benchmark/benchmark.h index 5e622fb61d..744653a08a 100644 --- a/mindspore/lite/tools/benchmark/benchmark.h +++ b/mindspore/lite/tools/benchmark/benchmark.h @@ -193,7 +193,7 @@ class MS_API Benchmark { auto tolerance = absoluteTolerance + relativeTolerance * fabs(calibTensor->data.at(j)); auto absoluteError = std::fabs(msTensorData[j] - calibTensor->data.at(j)); if (absoluteError > tolerance) { - if (fabs(calibTensor->data.at(j)) == 0) { + if (fabs(calibTensor->data.at(j) - 0.0f) < FLT_EPSILON) { if (absoluteError > 1e-5) { meanError += absoluteError; errorCount++; diff --git a/mindspore/lite/tools/converter/legacy_optimizer/fusion/matmul_biasadd_fusion_pass.cc b/mindspore/lite/tools/converter/legacy_optimizer/fusion/matmul_biasadd_fusion_pass.cc index 759b2cc8ae..be744dc3d5 100644 --- a/mindspore/lite/tools/converter/legacy_optimizer/fusion/matmul_biasadd_fusion_pass.cc +++ b/mindspore/lite/tools/converter/legacy_optimizer/fusion/matmul_biasadd_fusion_pass.cc @@ -155,7 +155,6 @@ STATUS MatMulBiasAddFusionPass::InsertTransposeNode(MetaGraphT *graph, const std MS_LOG(ERROR) << "new transposeParam failed"; return RET_ERROR; } - transposeParam->conjugate = false; transposeParam->perm = {1, 0}; transNode->primitive->value.value = transposeParam.release(); matmulOpIter = diff --git a/mindspore/lite/tools/converter/legacy_optimizer/fusion/matmul_biasadd_fusion_pass.h b/mindspore/lite/tools/converter/legacy_optimizer/fusion/matmul_biasadd_fusion_pass.h index 7350058ddc..671c2278b9 100644 --- a/mindspore/lite/tools/converter/legacy_optimizer/fusion/matmul_biasadd_fusion_pass.h +++ b/mindspore/lite/tools/converter/legacy_optimizer/fusion/matmul_biasadd_fusion_pass.h @@ -53,13 +53,18 @@ class MatMulBiasAddFusionPass : public FusionPass { size_t id = 0; OpDefCopyer TransposeOpCopyer = [](CNodeT *inOpDef) -> std::unique_ptr { - std::unique_ptr newOpDef(new (std::nothrow) CNodeT); + auto newOpDef = std::make_unique(); if (newOpDef == nullptr) { - MS_LOG(ERROR) << "new OpDefT failed"; + MS_LOG(ERROR) << "new CNodeT failed"; return nullptr; } newOpDef->name = inOpDef->name; newOpDef->quantType = inOpDef->quantType; + newOpDef->primitive = std::make_unique(); + if (newOpDef->primitive == nullptr) { + MS_LOG(ERROR) << "new PrimitiveT failed"; + return nullptr; + } newOpDef->primitive->value.type = schema::PrimitiveType_Transpose; auto transposeParam = new (std::nothrow) TransposeT; if (transposeParam == nullptr) { @@ -68,7 +73,6 @@ class MatMulBiasAddFusionPass : public FusionPass { } auto inParam = inOpDef->primitive->value.AsTranspose(); MS_ASSERT(inParam != nullptr); - transposeParam->conjugate = inParam->conjugate; transposeParam->perm.resize(inParam->perm.size()); std::transform(inParam->perm.begin(), inParam->perm.end(), transposeParam->perm.begin(), [](const int32_t ele) { return ele; }); diff --git a/mindspore/lite/tools/converter/legacy_optimizer/graph/format_trans_pass.h b/mindspore/lite/tools/converter/legacy_optimizer/graph/format_trans_pass.h index bb79bc3b50..14f4e1ab59 100644 --- a/mindspore/lite/tools/converter/legacy_optimizer/graph/format_trans_pass.h +++ b/mindspore/lite/tools/converter/legacy_optimizer/graph/format_trans_pass.h @@ -23,7 +23,7 @@ namespace mindspore { namespace lite { -enum FormatTransNodeType { kNCHW2NHWC, kNHWC2NCHW }; +enum FormatTransNodeType { kNCHW2NHWC, kNHWC2NCHW, kNONE }; class FormatTransPass : public GraphPass { public: diff --git a/mindspore/lite/tools/converter/parser/caffe/caffe_permute_parser.cc b/mindspore/lite/tools/converter/parser/caffe/caffe_permute_parser.cc index e552e4c34e..eeafed06d8 100644 --- a/mindspore/lite/tools/converter/parser/caffe/caffe_permute_parser.cc +++ b/mindspore/lite/tools/converter/parser/caffe/caffe_permute_parser.cc @@ -33,7 +33,6 @@ PrimitiveC *CaffePermuteParser::ParseLitePrimitive(const caffe::LayerParameter & for (int i = 0; i < num_order_dims; ++i) { attr->perm[i] = (int32_t)permuteParam.order()[i]; } - attr->conjugate = false; auto primitive = std::make_unique(); primitive->value.type = schema::PrimitiveType_Transpose; primitive->value.value = attr.release(); diff --git a/mindspore/lite/tools/converter/parser/onnx/onnx_transpose_parser.cc b/mindspore/lite/tools/converter/parser/onnx/onnx_transpose_parser.cc index afb2b8cf72..c481abbc19 100644 --- a/mindspore/lite/tools/converter/parser/onnx/onnx_transpose_parser.cc +++ b/mindspore/lite/tools/converter/parser/onnx/onnx_transpose_parser.cc @@ -28,7 +28,6 @@ lite::PrimitiveC *OnnxTransposeParser::ParseLitePrimitive(const onnx::GraphProto return nullptr; } - attr->conjugate = false; for (const auto &onnx_node_attr : onnx_node.attribute()) { const auto &attribute_name = onnx_node_attr.name(); if (attribute_name == "axes" || attribute_name == "perm") { diff --git a/mindspore/lite/tools/converter/parser/tf/tf_transpose_parser.cc b/mindspore/lite/tools/converter/parser/tf/tf_transpose_parser.cc index 6698d48404..5d8e7a3ec1 100644 --- a/mindspore/lite/tools/converter/parser/tf/tf_transpose_parser.cc +++ b/mindspore/lite/tools/converter/parser/tf/tf_transpose_parser.cc @@ -42,7 +42,6 @@ STATUS TFTransposeParser::Parse(const tensorflow::NodeDef &tf_op, return RET_NULL_PTR; } - attr->conjugate = false; if (tf_node_map.find(tf_op.input(1)) == tf_node_map.end()) { MS_LOG(ERROR) << "Find Transpose input perm failed"; return RET_ERROR; diff --git a/mindspore/lite/tools/converter/parser/tflite/tflite_transpose_parser.cc b/mindspore/lite/tools/converter/parser/tflite/tflite_transpose_parser.cc index 84aec3944c..b105a39e61 100644 --- a/mindspore/lite/tools/converter/parser/tflite/tflite_transpose_parser.cc +++ b/mindspore/lite/tools/converter/parser/tflite/tflite_transpose_parser.cc @@ -40,7 +40,6 @@ PrimitiveC *TfliteTransposeParser::ParseLitePrimitive(const std::unique_ptrconjugate = false; primitive->value.type = schema::PrimitiveType_Transpose; primitive->value.value = attr.release(); return PrimitiveC::Create(primitive.release()); diff --git a/mindspore/lite/tools/converter/parser/tflite/tflite_util.cc b/mindspore/lite/tools/converter/parser/tflite/tflite_util.cc index 22a1c345ae..d9f37a447e 100644 --- a/mindspore/lite/tools/converter/parser/tflite/tflite_util.cc +++ b/mindspore/lite/tools/converter/parser/tflite/tflite_util.cc @@ -210,7 +210,7 @@ STATUS getPaddingParam(const std::unique_ptr &tensor, schema::P return RET_ERROR; } if (tensor->shape.empty()) { - MS_LOG(DEBUG) << "the tensor's shape is dynamic, which obtain nly when running."; + MS_LOG(DEBUG) << "the tensor's shape is dynamic, which obtain only when running."; return RET_NO_CHANGE; } int padUp = 0;