From 8b7f45a8895a42cdd3e69576f9c9b3e4dce86245 Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Wed, 24 Oct 2018 11:27:23 +0800 Subject: [PATCH 01/12] add longs in framework --- paddle/fluid/framework/framework.proto | 2 ++ paddle/fluid/framework/op_desc.cc | 5 +++++ paddle/fluid/framework/type_defs.h | 2 +- paddle/fluid/pybind/protobuf.cc | 2 ++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/paddle/fluid/framework/framework.proto b/paddle/fluid/framework/framework.proto index c99406799b..2545e6c6f1 100644 --- a/paddle/fluid/framework/framework.proto +++ b/paddle/fluid/framework/framework.proto @@ -35,6 +35,7 @@ enum AttrType { BLOCK = 8; LONG = 9; BLOCKS = 10; + LONGS = 11; } // OpDesc describes an instance of a C++ framework::OperatorBase @@ -55,6 +56,7 @@ message OpDesc { optional int32 block_idx = 12; optional int64 l = 13; repeated int32 blocks_idx = 14; + optional int64 longs = 15; }; message Var { diff --git a/paddle/fluid/framework/op_desc.cc b/paddle/fluid/framework/op_desc.cc index c293cf92b4..7f81fb8641 100644 --- a/paddle/fluid/framework/op_desc.cc +++ b/paddle/fluid/framework/op_desc.cc @@ -421,6 +421,11 @@ struct SetAttrDescVisitor : public boost::static_visitor { } void operator()(BlockDesc *desc) const { attr_->set_block_idx(desc->ID()); } void operator()(int64_t v) const { attr_->set_l(v); } + + void operator()(const std::vector &v) const { + VectorToRepeated(v, attr_->mutable_longs()); + } + void operator()(boost::blank) const { PADDLE_THROW("Unexpected branch"); } }; diff --git a/paddle/fluid/framework/type_defs.h b/paddle/fluid/framework/type_defs.h index e099e40f12..2de6233a9e 100644 --- a/paddle/fluid/framework/type_defs.h +++ b/paddle/fluid/framework/type_defs.h @@ -36,7 +36,7 @@ using Attribute = boost::variant, std::vector, std::vector, bool, std::vector, BlockDesc*, int64_t, - std::vector>; + std::vector, std::vector>; using AttributeMap = std::unordered_map; diff --git a/paddle/fluid/pybind/protobuf.cc b/paddle/fluid/pybind/protobuf.cc index 3b22718a8c..0edfbfaa87 100644 --- a/paddle/fluid/pybind/protobuf.cc +++ b/paddle/fluid/pybind/protobuf.cc @@ -259,6 +259,8 @@ void BindOpDesc(pybind11::module *m) { pybind11::enum_(*m, "AttrType", "") .value("INT", pd::proto::AttrType::INT) .value("INTS", pd::proto::AttrType::INTS) + .value("LONG", pd::proto::AttrType::FLOAT) + .value("LONGS", pd::proto::AttrType::FLOAT) .value("FLOAT", pd::proto::AttrType::FLOAT) .value("FLOATS", pd::proto::AttrType::FLOATS) .value("STRING", pd::proto::AttrType::STRING) From 755927d2b00e101eebe5f619dc23fed56eae00ad Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Wed, 24 Oct 2018 20:33:02 +0800 Subject: [PATCH 02/12] shape type to int64_t, test=develop --- paddle/fluid/framework/framework.proto | 42 ++++++++++---------- paddle/fluid/framework/op_desc.cc | 6 ++- paddle/fluid/framework/type_defs.h | 8 ++-- paddle/fluid/operators/fill_constant_op.cc | 9 +++-- paddle/fluid/operators/gaussian_random_op.cc | 8 ++-- paddle/fluid/operators/uniform_random_op.cc | 6 +-- paddle/fluid/pybind/protobuf.cc | 4 +- 7 files changed, 43 insertions(+), 40 deletions(-) diff --git a/paddle/fluid/framework/framework.proto b/paddle/fluid/framework/framework.proto index 2545e6c6f1..423fe5e69b 100644 --- a/paddle/fluid/framework/framework.proto +++ b/paddle/fluid/framework/framework.proto @@ -25,17 +25,17 @@ message Version { optional int64 version = 1 [ default = 0 ]; } enum AttrType { INT = 0; - FLOAT = 1; - STRING = 2; - INTS = 3; - FLOATS = 4; - STRINGS = 5; - BOOLEAN = 6; - BOOLEANS = 7; - BLOCK = 8; - LONG = 9; - BLOCKS = 10; - LONGS = 11; + LONG = 1; + FLOAT = 2; + STRING = 3; + INTS = 4; + LONGS = 5; + FLOATS = 6; + STRINGS = 7; + BOOLEAN = 8; + BOOLEANS = 9; + BLOCK = 10; + BLOCKS = 11; } // OpDesc describes an instance of a C++ framework::OperatorBase @@ -46,17 +46,17 @@ message OpDesc { required string name = 1; required AttrType type = 2; optional int32 i = 3; - optional float f = 4; - optional string s = 5; - repeated int32 ints = 6; - repeated float floats = 7; - repeated string strings = 8; - optional bool b = 10; - repeated bool bools = 11; - optional int32 block_idx = 12; - optional int64 l = 13; + optional int64 l = 4; + optional float f = 5; + optional string s = 6; + repeated int32 ints = 7; + repeated int64 longs = 8; + repeated float floats = 9; + repeated string strings = 10; + optional bool b = 11; + repeated bool bools = 12; + optional int32 block_idx = 13; repeated int32 blocks_idx = 14; - optional int64 longs = 15; }; message Var { diff --git a/paddle/fluid/framework/op_desc.cc b/paddle/fluid/framework/op_desc.cc index 7f81fb8641..29b0061258 100644 --- a/paddle/fluid/framework/op_desc.cc +++ b/paddle/fluid/framework/op_desc.cc @@ -415,11 +415,13 @@ struct SetAttrDescVisitor : public boost::static_visitor { void operator()(const std::vector &v) const { std::vector blocks_idx; for (auto blk : v) { - blocks_idx.push_back(blk->ID()); + blocks_idx.push_sback(blk->ID()); } VectorToRepeated(blocks_idx, attr_->mutable_blocks_idx()); } - void operator()(BlockDesc *desc) const { attr_->set_block_idx(desc->ID()); } + void operator()(BlockDesapply_visitorc *desc) const { + attr_->set_block_idx(desc->ID()); + } void operator()(int64_t v) const { attr_->set_l(v); } void operator()(const std::vector &v) const { diff --git a/paddle/fluid/framework/type_defs.h b/paddle/fluid/framework/type_defs.h index 2de6233a9e..1cbf6c32ab 100644 --- a/paddle/fluid/framework/type_defs.h +++ b/paddle/fluid/framework/type_defs.h @@ -33,10 +33,10 @@ using VariableNameMap = std::map>; // The order should be as same as framework.proto using Attribute = - boost::variant, - std::vector, std::vector, bool, - std::vector, BlockDesc*, int64_t, - std::vector, std::vector>; + boost::variant, std::vector, std::vector, + std::vector, bool, std::vector, + BlockDesc*, std::vector>; using AttributeMap = std::unordered_map; diff --git a/paddle/fluid/operators/fill_constant_op.cc b/paddle/fluid/operators/fill_constant_op.cc index e04a68717b..252f313440 100644 --- a/paddle/fluid/operators/fill_constant_op.cc +++ b/paddle/fluid/operators/fill_constant_op.cc @@ -24,7 +24,7 @@ class FillConstantInferShape : public framework::InferShapeBase { void operator()(framework::InferShapeContext *ctx) const override { PADDLE_ENFORCE(ctx->HasOutput("Out"), "Output(Out) of FillConstantOp should not be null."); - auto &shape = ctx->Attrs().Get>("shape"); + auto &shape = ctx->Attrs().Get>("shape"); ctx->SetOutputDim("Out", framework::make_ddim(shape)); } }; @@ -47,10 +47,10 @@ class FillConstantOp : public framework::OperatorBase { if (out_var.IsType()) { tensor = out_var.GetMutable(); - tensor->Resize(framework::make_ddim(Attr>("shape"))); + tensor->Resize(framework::make_ddim(Attr>("shape"))); } else if (out_var.IsType()) { tensor = out_var.GetMutable()->mutable_value(); - tensor->Resize(framework::make_ddim(Attr>("shape"))); + tensor->Resize(framework::make_ddim(Attr>("shape"))); } else { PADDLE_THROW( "fill constant op's output only" @@ -83,7 +83,8 @@ class FillConstantOpMaker : public framework::OpProtoAndCheckerMaker { "(int, default 5 (FP32)) " "Output data type") .SetDefault(framework::proto::VarType::FP32); - AddAttr>("shape", "(vector) The shape of the output"); + AddAttr>("shape", + "(vector) The shape of the output"); AddAttr("value", "(float, default 0) The value to be filled") .SetDefault(0.0f); AddAttr("force_cpu", diff --git a/paddle/fluid/operators/gaussian_random_op.cc b/paddle/fluid/operators/gaussian_random_op.cc index 1488aab192..c70d5b8bc7 100644 --- a/paddle/fluid/operators/gaussian_random_op.cc +++ b/paddle/fluid/operators/gaussian_random_op.cc @@ -52,7 +52,7 @@ class GaussianRandomOp : public framework::OperatorWithKernel { void InferShape(framework::InferShapeContext* ctx) const override { PADDLE_ENFORCE(ctx->HasOutput("Out"), "Output(Out) of GaussianRandomOp should not be null."); - auto shape = ctx->Attrs().Get>("shape"); + auto shape = ctx->Attrs().Get>("shape"); std::vector temp; temp.reserve(shape.size()); for (auto dim : shape) { @@ -88,9 +88,9 @@ class GaussianRandomOpMaker : public framework::OpProtoAndCheckerMaker { void Make() override { AddOutput("Out", "Output matrix of gaussian random op"); - AddAttr>("shape", - "(vector) " - "The dimension of random tensor."); + AddAttr>("shape", + "(vector) " + "The dimension of random tensor."); AddAttr("mean", "(float, default 0.0) " "mean of random tensor.") diff --git a/paddle/fluid/operators/uniform_random_op.cc b/paddle/fluid/operators/uniform_random_op.cc index aa907595cb..e3132ae76f 100644 --- a/paddle/fluid/operators/uniform_random_op.cc +++ b/paddle/fluid/operators/uniform_random_op.cc @@ -29,7 +29,7 @@ class CPUUniformRandomKernel : public framework::OpKernel { if (out_var->IsType()) { tensor = out_var->GetMutable(); } else if (out_var->IsType()) { - auto shape = ctx.Attr>("shape"); + auto shape = ctx.Attr>("shape"); auto *selected_rows = out_var->GetMutable(); tensor = selected_rows->mutable_value(); tensor->Resize(framework::make_ddim(shape)); @@ -67,7 +67,7 @@ class UniformRandomOp : public framework::OperatorWithKernel { PADDLE_ENFORCE( ctx->Attrs().Get("min") < ctx->Attrs().Get("max"), "uniform_random's min must less then max"); - auto &shape = ctx->Attrs().Get>("shape"); + auto &shape = ctx->Attrs().Get>("shape"); std::vector temp; temp.reserve(shape.size()); for (auto dim : shape) { @@ -94,7 +94,7 @@ This operator initializes a tensor with random values sampled from a uniform distribution. The random result is in set [min, max]. )DOC"); - AddAttr>("shape", "The shape of the output tensor"); + AddAttr>("shape", "The shape of the output tensor"); AddAttr("min", "Minimum value of uniform random. [default -1.0].") .SetDefault(-1.0f); AddAttr("max", "Maximun value of uniform random. [default 1.0].") diff --git a/paddle/fluid/pybind/protobuf.cc b/paddle/fluid/pybind/protobuf.cc index 0edfbfaa87..cbc83106fc 100644 --- a/paddle/fluid/pybind/protobuf.cc +++ b/paddle/fluid/pybind/protobuf.cc @@ -259,8 +259,8 @@ void BindOpDesc(pybind11::module *m) { pybind11::enum_(*m, "AttrType", "") .value("INT", pd::proto::AttrType::INT) .value("INTS", pd::proto::AttrType::INTS) - .value("LONG", pd::proto::AttrType::FLOAT) - .value("LONGS", pd::proto::AttrType::FLOAT) + .value("LONG", pd::proto::AttrType::LONG) + .value("LONGS", pd::proto::AttrType::LONGS) .value("FLOAT", pd::proto::AttrType::FLOAT) .value("FLOATS", pd::proto::AttrType::FLOATS) .value("STRING", pd::proto::AttrType::STRING) From 39b3bf24d0a7b7758f70494adaccb9ba1e74d123 Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Wed, 24 Oct 2018 20:38:43 +0800 Subject: [PATCH 03/12] shape type to int64_t, test=develop --- paddle/fluid/framework/op_desc.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/paddle/fluid/framework/op_desc.cc b/paddle/fluid/framework/op_desc.cc index 29b0061258..7f81fb8641 100644 --- a/paddle/fluid/framework/op_desc.cc +++ b/paddle/fluid/framework/op_desc.cc @@ -415,13 +415,11 @@ struct SetAttrDescVisitor : public boost::static_visitor { void operator()(const std::vector &v) const { std::vector blocks_idx; for (auto blk : v) { - blocks_idx.push_sback(blk->ID()); + blocks_idx.push_back(blk->ID()); } VectorToRepeated(blocks_idx, attr_->mutable_blocks_idx()); } - void operator()(BlockDesapply_visitorc *desc) const { - attr_->set_block_idx(desc->ID()); - } + void operator()(BlockDesc *desc) const { attr_->set_block_idx(desc->ID()); } void operator()(int64_t v) const { attr_->set_l(v); } void operator()(const std::vector &v) const { From d1e85e33d72c94b19377dca6876fa7bc26bd25f9 Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Thu, 25 Oct 2018 14:53:42 +0800 Subject: [PATCH 04/12] shape type to int64_t, test=develop --- paddle/fluid/framework/attribute.h | 197 +++++++++++++++++------------ paddle/fluid/framework/op_desc.cc | 6 +- 2 files changed, 119 insertions(+), 84 deletions(-) diff --git a/paddle/fluid/framework/attribute.h b/paddle/fluid/framework/attribute.h index 14ca3e9620..d9c76881b7 100644 --- a/paddle/fluid/framework/attribute.h +++ b/paddle/fluid/framework/attribute.h @@ -26,6 +26,113 @@ limitations under the License. */ namespace paddle { namespace framework { + +template +struct ExtractAttribute { + explicit ExtractAttribute(const std::string& attr_name) + : attr_name_(attr_name) {} + + T* operator()(Attribute& attr) const { + T* attr_value = nullptr; + try { + attr_value = &boost::get(attr); + } catch (boost::bad_get& bad_get) { + PADDLE_THROW("Cannot get attribute %s by type %s, its type is %s", + attr_name_, paddle::platform::demangle(typeid(T).name()), + paddle::platform::demangle(attr.type().name())); + } + return attr_value; + } + + const std::string& attr_name_; +}; + +// special handle bool +// FIXME(yuyang18): Currently we cast bool into int in python binding. It is +// hard to change the logic there. In another way, we should correct handle +// if the user set `some_flag=1`. +// +// FIX ME anytime if there is a better solution. +template <> +struct ExtractAttribute { + explicit ExtractAttribute(const std::string& attr_name) + : attr_name_(attr_name) {} + + bool* operator()(Attribute& attr) const { + if (attr.type() == typeid(int)) { // NOLINT + int val = boost::get(attr); + attr = static_cast(val); + } else if (attr.type() == typeid(float)) { // NOLINT + float val = boost::get(attr); + attr = static_cast(val); + } + bool* attr_value = nullptr; + try { + attr_value = &boost::get(attr); + } catch (boost::bad_get& bad_get) { + PADDLE_THROW("Cannot get attribute %s by type bool, its type is %s", + attr_name_, paddle::platform::demangle(attr.type().name())); + } + return attr_value; + } + + const std::string& attr_name_; +}; + +template <> +struct ExtractAttribute { + explicit ExtractAttribute(const std::string& attr_name) + : attr_name_(attr_name) {} + + int64_t* operator()(Attribute& attr) const { + if (attr.type() == typeid(int)) { // NOLINT + int val = boost::get(attr); + attr = static_cast(val); + } else if (attr.type() == typeid(float)) { // NOLINT + int val = boost::get(attr); + attr = static_cast(val); + } + int64_t* attr_value = nullptr; + try { + attr_value = &boost::get(attr); + } catch (boost::bad_get& bad_get) { + PADDLE_THROW("Cannot get attribute %s by type int64_t, its type is %s", + attr_name_, paddle::platform::demangle(attr.type().name())); + } + return attr_value; + } + + const std::string& attr_name_; +}; + +template <> +struct ExtractAttribute> { + explicit ExtractAttribute(const std::string& attr_name) + : attr_name_(attr_name) {} + + std::vector* operator()(Attribute& attr) const { + if (attr.type() == typeid(std::vector)) { // NOLINT + std::vector val = boost::get>(attr); + std::vector vec(val.begin(), val.end()); + attr = vec; + } else if (attr.type() == typeid(std::vector)) { // NOLINT + std::vector val = boost::get>(attr); + std::vector vec(val.begin(), val.end()); + attr = vec; + } + std::vector* attr_value = nullptr; + try { + attr_value = &boost::get>(attr); + } catch (boost::bad_get& bad_get) { + PADDLE_THROW("Cannot get attribute %s by type int64_t, its type is %s", + attr_name_, paddle::platform::demangle(attr.type().name())); + } + return attr_value; + } + + const std::string& attr_name_; +}; + template inline proto::AttrType AttrTypeID() { Attribute tmp = T(); @@ -42,7 +149,11 @@ class AttrReader { inline const T& Get(const std::string& name) const { PADDLE_ENFORCE(attrs_.count(name) != 0, "%s should be in AttributeMap", name); - return boost::get(attrs_.at(name)); + + Attribute& attr = const_cast(attrs_.at(name)); + ExtractAttribute extract_attr(name); + T* attr_value = extract_attr(attr); + return *attr_value; } private: @@ -82,7 +193,7 @@ class DefaultValueSetter { public: explicit DefaultValueSetter(T default_value) : default_value_(default_value) {} - void operator()(T& value) const { value = default_value_; } + void operator()(T& value) const { value = default_value_; } // NOLINT private: T default_value_; @@ -117,84 +228,6 @@ class EnumInContainer { std::unordered_set container_; }; -template -struct ExtractAttribute { - explicit ExtractAttribute(const std::string& attr_name) - : attr_name_(attr_name) {} - - T* operator()(Attribute& attr) const { - T* attr_value = nullptr; - try { - attr_value = &boost::get(attr); - } catch (boost::bad_get& bad_get) { - PADDLE_THROW("Cannot get attribute %s by type %s, its type is %s", - attr_name_, paddle::platform::demangle(typeid(T).name()), - paddle::platform::demangle(attr.type().name())); - } - return attr_value; - } - - const std::string& attr_name_; -}; - -// special handle bool -// FIXME(yuyang18): Currently we cast bool into int in python binding. It is -// hard to change the logic there. In another way, we should correct handle -// if the user set `some_flag=1`. -// -// FIX ME anytime if there is a better solution. -template <> -struct ExtractAttribute { - explicit ExtractAttribute(const std::string& attr_name) - : attr_name_(attr_name) {} - - bool* operator()(Attribute& attr) const { - if (attr.type() == typeid(int)) { // NOLINT - int val = boost::get(attr); - attr = static_cast(val); - } else if (attr.type() == typeid(float)) { // NOLINT - float val = boost::get(attr); - attr = static_cast(val); - } - bool* attr_value = nullptr; - try { - attr_value = &boost::get(attr); - } catch (boost::bad_get& bad_get) { - PADDLE_THROW("Cannot get attribute %s by type bool, its type is %s", - attr_name_, paddle::platform::demangle(attr.type().name())); - } - return attr_value; - } - - const std::string& attr_name_; -}; - -template <> -struct ExtractAttribute { - explicit ExtractAttribute(const std::string& attr_name) - : attr_name_(attr_name) {} - - int64_t* operator()(Attribute& attr) const { - if (attr.type() == typeid(int)) { // NOLINT - int val = boost::get(attr); - attr = static_cast(val); - } else if (attr.type() == typeid(float)) { // NOLINT - int val = boost::get(attr); - attr = static_cast(val); - } - int64_t* attr_value = nullptr; - try { - attr_value = &boost::get(attr); - } catch (boost::bad_get& bad_get) { - PADDLE_THROW("Cannot get attribute %s by type int64_t, its type is %s", - attr_name_, paddle::platform::demangle(attr.type().name())); - } - return attr_value; - } - - const std::string& attr_name_; -}; - // check whether a certain attribute fit its limits // an attribute can have more than one limits template @@ -235,7 +268,7 @@ class TypedAttrChecker { return *this; } - void operator()(AttributeMap& attr_map) const { + void operator()(AttributeMap& attr_map) const { // NOLINT if (!attr_map.count(attr_name_)) { // user do not set this attr PADDLE_ENFORCE(!default_value_setter_.empty(), @@ -271,7 +304,7 @@ class OpAttrChecker { return *(checker.target>()); } - void Check(AttributeMap& attr_map) const { + void Check(AttributeMap& attr_map) const { // NOLINT for (const auto& checker : attr_checkers_) { checker(attr_map); } diff --git a/paddle/fluid/framework/op_desc.cc b/paddle/fluid/framework/op_desc.cc index 7f81fb8641..29b0061258 100644 --- a/paddle/fluid/framework/op_desc.cc +++ b/paddle/fluid/framework/op_desc.cc @@ -415,11 +415,13 @@ struct SetAttrDescVisitor : public boost::static_visitor { void operator()(const std::vector &v) const { std::vector blocks_idx; for (auto blk : v) { - blocks_idx.push_back(blk->ID()); + blocks_idx.push_sback(blk->ID()); } VectorToRepeated(blocks_idx, attr_->mutable_blocks_idx()); } - void operator()(BlockDesc *desc) const { attr_->set_block_idx(desc->ID()); } + void operator()(BlockDesapply_visitorc *desc) const { + attr_->set_block_idx(desc->ID()); + } void operator()(int64_t v) const { attr_->set_l(v); } void operator()(const std::vector &v) const { From 0e25e397bdf71ef6e522dbdb6d3de991b5bc019c Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Thu, 25 Oct 2018 15:01:10 +0800 Subject: [PATCH 05/12] shape type to int64_t, test=develop --- paddle/fluid/framework/op_desc.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/paddle/fluid/framework/op_desc.cc b/paddle/fluid/framework/op_desc.cc index 29b0061258..8ece618f3f 100644 --- a/paddle/fluid/framework/op_desc.cc +++ b/paddle/fluid/framework/op_desc.cc @@ -415,13 +415,13 @@ struct SetAttrDescVisitor : public boost::static_visitor { void operator()(const std::vector &v) const { std::vector blocks_idx; for (auto blk : v) { - blocks_idx.push_sback(blk->ID()); + blocks_idx.push_back(blk->ID()); } VectorToRepeated(blocks_idx, attr_->mutable_blocks_idx()); } - void operator()(BlockDesapply_visitorc *desc) const { - attr_->set_block_idx(desc->ID()); - } + + void operator()(BlockDesc *desc) const { attr_->set_block_idx(desc->ID()); } + void operator()(int64_t v) const { attr_->set_l(v); } void operator()(const std::vector &v) const { From d4a8967c1e2b50a7dda517427ac0d2aa5dd5f8ef Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Thu, 25 Oct 2018 16:10:10 +0800 Subject: [PATCH 06/12] add const in &, test=develop --- paddle/fluid/framework/attribute.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/paddle/fluid/framework/attribute.h b/paddle/fluid/framework/attribute.h index d9c76881b7..f3ad88626f 100644 --- a/paddle/fluid/framework/attribute.h +++ b/paddle/fluid/framework/attribute.h @@ -193,7 +193,7 @@ class DefaultValueSetter { public: explicit DefaultValueSetter(T default_value) : default_value_(default_value) {} - void operator()(T& value) const { value = default_value_; } // NOLINT + void operator()(const T& value) const { value = default_value_; } private: T default_value_; @@ -268,7 +268,7 @@ class TypedAttrChecker { return *this; } - void operator()(AttributeMap& attr_map) const { // NOLINT + void operator()(const AttributeMap& attr_map) const { if (!attr_map.count(attr_name_)) { // user do not set this attr PADDLE_ENFORCE(!default_value_setter_.empty(), @@ -294,7 +294,7 @@ class TypedAttrChecker { // check whether op's all attributes fit their own limits class OpAttrChecker { - typedef std::function AttrChecker; + typedef std::function AttrChecker; public: template @@ -304,7 +304,7 @@ class OpAttrChecker { return *(checker.target>()); } - void Check(AttributeMap& attr_map) const { // NOLINT + void Check(const AttributeMap& attr_map) const { for (const auto& checker : attr_checkers_) { checker(attr_map); } From 2761eafb923c29db0f78bd20ae3d81c6d7cae60a Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Thu, 25 Oct 2018 16:17:12 +0800 Subject: [PATCH 07/12] shape type to int64_t, test=develop --- paddle/fluid/framework/attribute.cc | 7 +++++++ paddle/fluid/framework/attribute.h | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/paddle/fluid/framework/attribute.cc b/paddle/fluid/framework/attribute.cc index 0dcecb62db..fabf2abfc8 100644 --- a/paddle/fluid/framework/attribute.cc +++ b/paddle/fluid/framework/attribute.cc @@ -64,6 +64,13 @@ Attribute GetAttrValue(const proto::OpDesc::Attr& attr_desc) { case proto::AttrType::LONG: { return attr_desc.l(); } + case proto::AttrType::LONGS: { + std::vector val(attr_desc.longs_size()); + for (int i = 0; i < attr_desc.longs_size(); ++i) { + val[i] = attr_desc.longs(i); + } + return val; + } default: PADDLE_THROW("Unsupport attr type %d", attr_desc.type()); } diff --git a/paddle/fluid/framework/attribute.h b/paddle/fluid/framework/attribute.h index f3ad88626f..d9c76881b7 100644 --- a/paddle/fluid/framework/attribute.h +++ b/paddle/fluid/framework/attribute.h @@ -193,7 +193,7 @@ class DefaultValueSetter { public: explicit DefaultValueSetter(T default_value) : default_value_(default_value) {} - void operator()(const T& value) const { value = default_value_; } + void operator()(T& value) const { value = default_value_; } // NOLINT private: T default_value_; @@ -268,7 +268,7 @@ class TypedAttrChecker { return *this; } - void operator()(const AttributeMap& attr_map) const { + void operator()(AttributeMap& attr_map) const { // NOLINT if (!attr_map.count(attr_name_)) { // user do not set this attr PADDLE_ENFORCE(!default_value_setter_.empty(), @@ -294,7 +294,7 @@ class TypedAttrChecker { // check whether op's all attributes fit their own limits class OpAttrChecker { - typedef std::function AttrChecker; + typedef std::function AttrChecker; public: template @@ -304,7 +304,7 @@ class OpAttrChecker { return *(checker.target>()); } - void Check(const AttributeMap& attr_map) const { + void Check(AttributeMap& attr_map) const { // NOLINT for (const auto& checker : attr_checkers_) { checker(attr_map); } From b58957d9d792b8ec85ad460a02ecc1f13575e7cd Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Thu, 25 Oct 2018 17:08:40 +0800 Subject: [PATCH 08/12] Revert "fix lookuptable in reduce strategy" This reverts commit 0e722c5 --- paddle/fluid/framework/details/multi_devices_graph_pass.cc | 3 +-- paddle/fluid/framework/ir/graph.cc | 6 ------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/paddle/fluid/framework/details/multi_devices_graph_pass.cc b/paddle/fluid/framework/details/multi_devices_graph_pass.cc index 4f481db061..134fcee826 100644 --- a/paddle/fluid/framework/details/multi_devices_graph_pass.cc +++ b/paddle/fluid/framework/details/multi_devices_graph_pass.cc @@ -680,8 +680,7 @@ int MultiDevSSAGraphBuilder::CreateDistTrainOp(ir::Graph *result, } if (node->Op()->Type() == "split_byref" || - node->Op()->Type() == "split_selected_rows" || - node->Op()->Type() == "split_ids") { + node->Op()->Type() == "split_selected_rows") { // TODO(paddle-dev): getting the first var is not safe. op_dev_id = GetVarDeviceID(*result, input_var_names[0]); if (strategy_.reduce_ == BuildStrategy::ReduceStrategy::kAllReduce) { diff --git a/paddle/fluid/framework/ir/graph.cc b/paddle/fluid/framework/ir/graph.cc index 87fc5e6891..398f709596 100644 --- a/paddle/fluid/framework/ir/graph.cc +++ b/paddle/fluid/framework/ir/graph.cc @@ -69,12 +69,6 @@ bool IsDistTrainOp(ir::Node *node, const std::vector &send_vars, std::find(rpc_vars.begin(), rpc_vars.end(), var) != rpc_vars.end()) { return true; } - - if (!(var.find(".block") == std::string::npos && - var.find(".pserver") != std::string::npos) && - std::find(rpc_vars.begin(), rpc_vars.end(), var) != rpc_vars.end()) { - return true; - } } return false; }; From aa6dc82f4bdddc9fa9ded310c62eff40c03e101e Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Fri, 26 Oct 2018 13:00:44 +0800 Subject: [PATCH 09/12] revert changes in protobuf.cc and type_defs --- paddle/fluid/framework/framework.proto | 42 +++++++++++++------------- paddle/fluid/framework/type_defs.h | 8 ++--- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/paddle/fluid/framework/framework.proto b/paddle/fluid/framework/framework.proto index 423fe5e69b..2545e6c6f1 100644 --- a/paddle/fluid/framework/framework.proto +++ b/paddle/fluid/framework/framework.proto @@ -25,17 +25,17 @@ message Version { optional int64 version = 1 [ default = 0 ]; } enum AttrType { INT = 0; - LONG = 1; - FLOAT = 2; - STRING = 3; - INTS = 4; - LONGS = 5; - FLOATS = 6; - STRINGS = 7; - BOOLEAN = 8; - BOOLEANS = 9; - BLOCK = 10; - BLOCKS = 11; + FLOAT = 1; + STRING = 2; + INTS = 3; + FLOATS = 4; + STRINGS = 5; + BOOLEAN = 6; + BOOLEANS = 7; + BLOCK = 8; + LONG = 9; + BLOCKS = 10; + LONGS = 11; } // OpDesc describes an instance of a C++ framework::OperatorBase @@ -46,17 +46,17 @@ message OpDesc { required string name = 1; required AttrType type = 2; optional int32 i = 3; - optional int64 l = 4; - optional float f = 5; - optional string s = 6; - repeated int32 ints = 7; - repeated int64 longs = 8; - repeated float floats = 9; - repeated string strings = 10; - optional bool b = 11; - repeated bool bools = 12; - optional int32 block_idx = 13; + optional float f = 4; + optional string s = 5; + repeated int32 ints = 6; + repeated float floats = 7; + repeated string strings = 8; + optional bool b = 10; + repeated bool bools = 11; + optional int32 block_idx = 12; + optional int64 l = 13; repeated int32 blocks_idx = 14; + optional int64 longs = 15; }; message Var { diff --git a/paddle/fluid/framework/type_defs.h b/paddle/fluid/framework/type_defs.h index 1cbf6c32ab..2de6233a9e 100644 --- a/paddle/fluid/framework/type_defs.h +++ b/paddle/fluid/framework/type_defs.h @@ -33,10 +33,10 @@ using VariableNameMap = std::map>; // The order should be as same as framework.proto using Attribute = - boost::variant, std::vector, std::vector, - std::vector, bool, std::vector, - BlockDesc*, std::vector>; + boost::variant, + std::vector, std::vector, bool, + std::vector, BlockDesc*, int64_t, + std::vector, std::vector>; using AttributeMap = std::unordered_map; From 318ba99124331742b3e4e985a71bddcb074b70a1 Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Fri, 26 Oct 2018 13:08:22 +0800 Subject: [PATCH 10/12] revert changes in protobuf.cc and type_defs --- paddle/fluid/framework/framework.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paddle/fluid/framework/framework.proto b/paddle/fluid/framework/framework.proto index 2545e6c6f1..efdabffb9b 100644 --- a/paddle/fluid/framework/framework.proto +++ b/paddle/fluid/framework/framework.proto @@ -56,7 +56,7 @@ message OpDesc { optional int32 block_idx = 12; optional int64 l = 13; repeated int32 blocks_idx = 14; - optional int64 longs = 15; + repeated int64 longs = 15; }; message Var { From d8b697357f73c0d548b7745bb31524bcbc0580b1 Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Fri, 26 Oct 2018 17:03:07 +0800 Subject: [PATCH 11/12] update height_sections to int64_t --- paddle/fluid/operators/split_selected_rows_op.cc | 6 +++--- paddle/fluid/operators/split_selected_rows_op.h | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/paddle/fluid/operators/split_selected_rows_op.cc b/paddle/fluid/operators/split_selected_rows_op.cc index 76615a9405..0e7b1463d1 100644 --- a/paddle/fluid/operators/split_selected_rows_op.cc +++ b/paddle/fluid/operators/split_selected_rows_op.cc @@ -22,9 +22,9 @@ class SplitSelectedRowsOpMaker : public framework::OpProtoAndCheckerMaker { void Make() override { AddInput("X", "The input SelectedRows."); AddOutput("Out", "The outputs of the input SelectedRows.").AsDuplicable(); - AddAttr>("height_sections", - "Height for each output SelectedRows.") - .SetDefault(std::vector({})); + AddAttr>("height_sections", + "Height for each output SelectedRows.") + .SetDefault(std::vector({})); AddComment(R"DOC( Split a SelectedRows with a specified rows section. diff --git a/paddle/fluid/operators/split_selected_rows_op.h b/paddle/fluid/operators/split_selected_rows_op.h index 0e9ce165b9..af64607faf 100644 --- a/paddle/fluid/operators/split_selected_rows_op.h +++ b/paddle/fluid/operators/split_selected_rows_op.h @@ -21,7 +21,7 @@ limitations under the License. */ namespace paddle { namespace operators { -static int FindOutIdx(int row, const std::vector& abs_sections) { +static int FindOutIdx(int row, const std::vector& abs_sections) { for (size_t i = 1; i < abs_sections.size(); ++i) { if (row < abs_sections[i]) { return i - 1; @@ -30,9 +30,9 @@ static int FindOutIdx(int row, const std::vector& abs_sections) { return abs_sections.size() - 1; } -static std::vector ToAbsoluteSection( - const std::vector& height_sections) { - std::vector abs_sections; +static std::vector ToAbsoluteSection( + const std::vector& height_sections) { + std::vector abs_sections; abs_sections.resize(height_sections.size()); abs_sections[0] = 0; for (size_t i = 1; i < height_sections.size(); ++i) { @@ -47,7 +47,7 @@ class SplitSelectedRowsOpKernel : public framework::OpKernel { void Compute(const framework::ExecutionContext& ctx) const override { auto* x = ctx.Input("X"); auto outs = ctx.MultiOutput("Out"); - auto height_sections = ctx.Attr>("height_sections"); + auto height_sections = ctx.Attr>("height_sections"); auto abs_sections = ToAbsoluteSection(height_sections); From cb1ccc710b86805ee80b2da163407c6ad36b8f89 Mon Sep 17 00:00:00 2001 From: tangwei12 Date: Sun, 28 Oct 2018 09:00:54 +0800 Subject: [PATCH 12/12] fix shape type in uniform_random_op.cu --- paddle/fluid/operators/uniform_random_op.cu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paddle/fluid/operators/uniform_random_op.cu b/paddle/fluid/operators/uniform_random_op.cu index bbb692b0dd..2bb0ecc139 100644 --- a/paddle/fluid/operators/uniform_random_op.cu +++ b/paddle/fluid/operators/uniform_random_op.cu @@ -48,7 +48,7 @@ class GPUUniformRandomKernel : public framework::OpKernel { if (out_var->IsType()) { tensor = out_var->GetMutable(); } else if (out_var->IsType()) { - auto shape = context.Attr>("shape"); + auto shape = context.Attr>("shape"); tensor = out_var->GetMutable()->mutable_value(); tensor->Resize(framework::make_ddim(shape)); } else {