Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change operator public to protected #3460

Merged
merged 10 commits into from
Aug 15, 2017
28 changes: 14 additions & 14 deletions paddle/framework/backward.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace paddle {
namespace framework {

template <typename Map, typename T>
static void ForEachVarName(Map& names, T callback) {
static void ForEachVarName(const Map& names, T callback) {
for (auto& name : names) {
for (auto& n : name.second) {
if (callback(n)) return;
Expand All @@ -44,7 +44,7 @@ static bool AllInSet(

static std::shared_ptr<OperatorBase> NOP() {
auto net_op = std::make_shared<operators::NetOp>();
net_op->type_ = "@NOP@";
net_op->SetType("@NOP@");
net_op->CompleteAddOp();
return net_op;
}
Expand All @@ -70,18 +70,18 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
std::unordered_set<std::string>& no_grad_names, size_t& uniq_id) {
// If all input gradients of forwarding operator do not need to calculate,
// just return an NOP. Not return null ptr because NOP does not take
// much time for calculation, but it is useful for simplifying logic.
if (AllInSet(forwardOp.inputs_ /*names*/, kGradVarSuffix /*suffix*/,
// too much time for calculation, but it is useful for simplifying logic.
if (AllInSet(forwardOp.Inputs() /*names*/, kGradVarSuffix /*suffix*/,
no_grad_names /*set*/)) {
return NOP();
}

// All output gradients of forwarding operator do not need to calculate.
// Then all input gradients cannot be computed at all, and we put them into
// `no_grad_names` set. Return an NOP.
if (AllInSet(forwardOp.outputs_ /*names*/, kGradVarSuffix /*suffix*/,
if (AllInSet(forwardOp.Outputs() /*names*/, kGradVarSuffix /*suffix*/,
no_grad_names /*set*/)) {
ForEachVarName(forwardOp.inputs_,
ForEachVarName(forwardOp.Inputs(),
[&no_grad_names](const std::string& name) -> bool {
no_grad_names.insert(GradVarName(name));
return false;
Expand All @@ -107,7 +107,7 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
auto fwd = *it;
auto bwd = BackwardRecursive(*fwd, no_grad_names, uniq_id);
net->AddOp(bwd);
ForEachVarName(bwd->outputs_,
ForEachVarName(bwd->Outputs(),
[&dup_output_ops, local_op_id](const std::string& out) {
dup_output_ops[out].emplace_back(local_op_id);
return false;
Expand Down Expand Up @@ -154,13 +154,13 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
} else {
std::shared_ptr<OperatorBase> grad_op = OpRegistry::CreateGradOp(forwardOp);

ForEachVarName(grad_op->inputs_, [&no_grad_names,
&net](std::string& grad_input) {
ForEachVarName(grad_op->Inputs(), [&no_grad_names, &net,
grad_op](const std::string& grad_input) {
if (no_grad_names.count(grad_input)) {
// +1 for \0
std::string prefix = grad_input.substr(
0, grad_input.size() - sizeof(kGradVarSuffix) / sizeof(char) + 1);
grad_input = prefix + kZeroVarSuffix;
grad_op->Rename(grad_input, prefix + kZeroVarSuffix);

// If part of input gradient of that operator is not calculated, fill
// zero variables to that input gradient.
Expand All @@ -170,10 +170,10 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
return false;
});

ForEachVarName(grad_op->outputs_,
[&no_grad_names](std::string& grad_output) {
ForEachVarName(grad_op->Outputs(),
[&no_grad_names, &grad_op](const std::string& grad_output) {
if (no_grad_names.count(grad_output)) {
grad_output = kEmptyVarName;
grad_op->Rename(grad_output, kEmptyVarName);
}
return false;
});
Expand All @@ -183,7 +183,7 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
}
net->AddOp(grad_op);
}
net->type_ = "@GENERATED_BACKWARD@";
net->SetType("@GENERATED_BACKWARD@");
net->CompleteAddOp();
return net;
} // namespace framework
Expand Down
40 changes: 20 additions & 20 deletions paddle/framework/backward_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ TEST(Backward, simple_op_grad) {
"rowwise_add", {{"X", {"x"}}, {"b", {"b"}}}, {{"Out", {"out"}}}, {});
ASSERT_NE(fwd, nullptr);
auto gop = f::OpRegistry::CreateGradOp(*fwd);
ASSERT_EQ(1UL, gop->inputs_.size());
ASSERT_EQ("rowwise_add_grad", gop->type_);
ASSERT_EQ(1UL, gop->Inputs().size());
ASSERT_EQ("rowwise_add_grad", gop->Type());
ASSERT_EQ(f::GradVarName("x"), gop->Output(f::GradVarName("X")));
ASSERT_EQ(f::GradVarName("b"), gop->Output(f::GradVarName("b")));
}
Expand Down Expand Up @@ -211,13 +211,13 @@ TEST(Backward, net_fc_backward_normal) {
ASSERT_EQ(3UL, net->ops_.size());

f::OperatorBase &d_sigmoid = *net->ops_[0];
ASSERT_EQ("sigmoid_grad", d_sigmoid.type_);
ASSERT_EQ("sigmoid_grad", d_sigmoid.Type());

f::OperatorBase &d_add = *net->ops_[1];
ASSERT_EQ("rowwise_add_grad", d_add.type_);
ASSERT_EQ("rowwise_add_grad", d_add.Type());

f::OperatorBase &d_mul = *net->ops_[2];
ASSERT_EQ("mul_grad", d_mul.type_);
ASSERT_EQ("mul_grad", d_mul.Type());
}

TEST(Backward, net_fc_backward_not_have_b) {
Expand All @@ -237,10 +237,10 @@ TEST(Backward, net_fc_backward_not_have_b) {
ASSERT_EQ(2UL, net->ops_.size());

f::OperatorBase &d_sigmoid = *net->ops_[0];
ASSERT_EQ("sigmoid_grad", d_sigmoid.type_);
ASSERT_EQ("sigmoid_grad", d_sigmoid.Type());

f::OperatorBase &d_mul = *net->ops_[1];
ASSERT_EQ("mul_grad", d_mul.type_);
ASSERT_EQ("mul_grad", d_mul.Type());
}

TEST(Backward, net_input_of_network_not_need_grad) {
Expand Down Expand Up @@ -294,7 +294,7 @@ TEST(Backward, net_shared_weight) {
ASSERT_TRUE(bwd->IsNetOp());
auto bwd_net = static_cast<ops::NetOp *>(bwd.get());
ASSERT_EQ(3UL, bwd_net->ops_.size());
ASSERT_EQ("add", bwd_net->ops_[2]->type_);
ASSERT_EQ("add", bwd_net->ops_[2]->Type());
}

TEST(Backward, op_register_grad_not_for_network) {
Expand Down Expand Up @@ -335,15 +335,15 @@ TEST(Backward, op_part_of_output_are_not_need) {
ASSERT_EQ(net->ops_.size(), 2UL);

auto &fill_zero = *net->ops_[0];
ASSERT_EQ("fill_zeros_like", fill_zero.type_);
ASSERT_EQ("fill_zeros_like", fill_zero.Type());
ASSERT_EQ(1UL, fill_zero.Inputs("Src").size());
ASSERT_EQ("Z", fill_zero.Input("Src"));
ASSERT_EQ(1UL, fill_zero.Outputs("Dst").size());
ASSERT_EQ(std::string("Z") + f::kZeroVarSuffix, fill_zero.Output("Dst"));

auto &d_many_out = *net->ops_[1];
ASSERT_EQ("many_output_op_grad", d_many_out.type_);
ASSERT_EQ(1UL + 2UL + 2UL, d_many_out.inputs_.size()); // I/O/OG
ASSERT_EQ("many_output_op_grad", d_many_out.Type());
ASSERT_EQ(1UL + 2UL + 2UL, d_many_out.Inputs().size()); // I/O/OG
ASSERT_EQ(std::string("Z") + f::kZeroVarSuffix,
d_many_out.Input(f::GradVarName("z")));
ASSERT_EQ(f::GradVarName("Y"), d_many_out.Input(f::GradVarName("y")));
Expand All @@ -355,9 +355,9 @@ TEST(Backward, op_part_of_input_are_not_need) {
{{"Out", {"out"}}}, {});
auto backward = f::Backward(*fwd, {"a"});
auto &grad_mul = *backward;
ASSERT_EQ(grad_mul.type_, "mul_grad");
ASSERT_EQ(grad_mul.inputs_.size(), 2UL + 1UL + 1UL);
ASSERT_EQ(grad_mul.outputs_.size(), 2UL);
ASSERT_EQ(grad_mul.Type(), "mul_grad");
ASSERT_EQ(grad_mul.Inputs().size(), 2UL + 1UL + 1UL);
ASSERT_EQ(grad_mul.Outputs().size(), 2UL);
ASSERT_EQ(grad_mul.Output(f::GradVarName("X")), f::kEmptyVarName);
ASSERT_EQ(grad_mul.Output(f::GradVarName("Y")), f::GradVarName("b"));
ASSERT_EQ(grad_mul.Input(f::GradVarName("Out")), f::GradVarName("out"));
Expand Down Expand Up @@ -395,18 +395,18 @@ TEST(Backward, linear_net_intermediate_variable_has_no_grad) {
auto &grad_fc = *bwd_net->ops_[0];

const char *all = paddle::operators::NetOp::kAll;
EXPECT_EQ(grad_fc.inputs_[all].size(),
EXPECT_EQ(grad_fc.Inputs(all).size(),
2UL /* external input number */
+ 1UL /* external output number*/
+ 1UL /* number of gradient of external output*/
+ 2U /* internal variable number*/);
EXPECT_EQ(grad_fc.outputs_[all].size(),
EXPECT_EQ(grad_fc.Outputs(all).size(),
2UL /* input number of mul*/
+ 2UL /* input number of rowwise_add
*/
+ 1UL /* input number of sigmod */);
EXPECT_EQ(bwd_net->ops_[1]->inputs_[all].size(), 0UL);
EXPECT_EQ(bwd_net->ops_[1]->outputs_[all].size(), 0UL);
EXPECT_EQ(bwd_net->ops_[2]->inputs_[all].size(), 0UL);
EXPECT_EQ(bwd_net->ops_[2]->outputs_[all].size(), 0UL);
EXPECT_EQ(bwd_net->ops_[1]->Inputs(all).size(), 0UL);
EXPECT_EQ(bwd_net->ops_[1]->Outputs(all).size(), 0UL);
EXPECT_EQ(bwd_net->ops_[2]->Inputs(all).size(), 0UL);
EXPECT_EQ(bwd_net->ops_[2]->Outputs(all).size(), 0UL);
}
13 changes: 6 additions & 7 deletions paddle/framework/grad_op_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ express or implied. See the License for the specific language governing
permissions and limitations under the License. */

#include "paddle/framework/grad_op_builder.h"
#include "paddle/framework/framework.pb.h"
#include "paddle/framework/op_registry.h"

namespace paddle {
Expand All @@ -24,10 +23,10 @@ static void TransOpArg(const OperatorBase* src_op,
OperatorBase::VarNameMap* vars,
const OpArgType& src_type, bool is_grad) {
const auto& src_inout =
src_type == OpArgType::IN ? src_op->inputs_ : src_op->outputs_;
src_type == OpArgType::IN ? src_op->Inputs() : src_op->Outputs();
auto& dst_inout = *vars;

const OpProto& proto = OpProtos().at(src_op->type_);
const OpProto& proto = OpProtos().at(src_op->Type());
const auto& src_arg_list =
src_type == OpArgType::IN ? proto.inputs() : proto.outputs();
for (const auto& arg : src_arg_list) {
Expand All @@ -43,9 +42,9 @@ static void TransOpArg(const OperatorBase* src_op,
}

OperatorBase* BuildGradOp(const OperatorBase* op) {
auto gop_type_it = OpRegistry::grad_ops().find(op->type_);
auto gop_type_it = OpRegistry::grad_ops().find(op->Type());
PADDLE_ENFORCE(gop_type_it != OpRegistry::grad_ops().end(),
"Operator %s do not register gradient type", op->type_);
"Operator %s do not register gradient type", op->Type());
auto& grad_op_type = gop_type_it->second;
OperatorBase::VarNameMap inputs;
OperatorBase::VarNameMap outputs;
Expand All @@ -56,9 +55,9 @@ OperatorBase* BuildGradOp(const OperatorBase* op) {
auto gop_it = OpRegistry::op_creators().find(grad_op_type);
PADDLE_ENFORCE(gop_it != OpRegistry::op_creators().end(),
"Operator %s 's Gradient %s's creator cannot be found",
op->type_, grad_op_type);
op->Type(), grad_op_type);

return gop_it->second(grad_op_type, inputs, outputs, op->attrs_);
return gop_it->second(grad_op_type, inputs, outputs, op->Attrs());
}

} // namespace framework
Expand Down
12 changes: 6 additions & 6 deletions paddle/framework/grad_op_builder_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ TEST(GradOpBuilder, AddTwo) {
"add_two", {{"X", {"x"}}, {"Y", {"y"}}}, {{"Out", {"out"}}}, {}));
std::shared_ptr<f::OperatorBase> grad_add_op =
f::OpRegistry::CreateGradOp(*add_op);
EXPECT_EQ(grad_add_op->inputs_.size(), 4UL);
EXPECT_EQ(grad_add_op->outputs_.size(), 2UL);
EXPECT_EQ(grad_add_op->Inputs().size(), 4UL);
EXPECT_EQ(grad_add_op->Outputs().size(), 2UL);
EXPECT_EQ(grad_add_op->Input("X"), "x");
EXPECT_EQ(grad_add_op->Input("Y"), "y");
EXPECT_EQ(grad_add_op->Input("Out"), "out");
Expand All @@ -76,7 +76,7 @@ TEST(GradOpBuilder, MutiInOut) {
std::shared_ptr<f::OperatorBase> grad_test_op =
f::OpRegistry::CreateGradOp(*test_op);

ASSERT_EQ(grad_test_op->inputs_.size(), 3UL + 2UL + 2UL);
ASSERT_EQ(grad_test_op->Inputs().size(), 3UL + 2UL + 2UL);
EXPECT_EQ(grad_test_op->Input("In1"), "in1");
EXPECT_EQ(grad_test_op->Inputs("In2_mult"),
std::vector<std::string>({"in2_1", "in2_2", "in2_3"}));
Expand All @@ -90,7 +90,7 @@ TEST(GradOpBuilder, MutiInOut) {
std::vector<std::string>(
{f::GradVarName("out2_1"), f::GradVarName("out2_2")}));

ASSERT_EQ(grad_test_op->outputs_.size(), 3UL);
ASSERT_EQ(grad_test_op->Outputs().size(), 3UL);
EXPECT_EQ(grad_test_op->Output(f::GradVarName("In1")), f::GradVarName("in1"));
EXPECT_EQ(grad_test_op->Outputs(f::GradVarName("In2_mult")),
std::vector<std::string>({f::GradVarName("in2_1"),
Expand All @@ -109,7 +109,7 @@ TEST(GradOpBuilder, IOIgnoredInGradient) {
f::OpRegistry::CreateGradOp(*test_op);

// 'In2' and 'Out2' are ignored in gradient calculating
ASSERT_EQ(grad_test_op->inputs_.size(), 2UL + 1UL + 2UL);
ASSERT_EQ(grad_test_op->Inputs().size(), 2UL + 1UL + 2UL);
EXPECT_EQ(grad_test_op->Input("In1"), "in1");
EXPECT_EQ(grad_test_op->Inputs("In3_mult"),
std::vector<std::string>({"in3_1", "in3_2"}));
Expand All @@ -121,7 +121,7 @@ TEST(GradOpBuilder, IOIgnoredInGradient) {
EXPECT_EQ(grad_test_op->Input(f::GradVarName("Out2")),
f::GradVarName("out2"));

ASSERT_EQ(grad_test_op->outputs_.size(), 3UL);
ASSERT_EQ(grad_test_op->Outputs().size(), 3UL);
EXPECT_EQ(grad_test_op->Output(f::GradVarName("In1")), f::GradVarName("in1"));
EXPECT_EQ(grad_test_op->Outputs(f::GradVarName("In2_mult")),
std::vector<std::string>(
Expand Down
1 change: 0 additions & 1 deletion paddle/framework/op_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ class OpRegistry {
PADDLE_ENFORCE(op_create_it != op_creators().end(),
"Operator %s cannot be found.", type);
op_checkers().at(type).Check(attrs);

auto op = op_create_it->second(type, inputs, outputs, attrs);

return std::shared_ptr<OperatorBase>(op);
Expand Down
7 changes: 5 additions & 2 deletions paddle/framework/operator.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ class OperatorBase {
/// rename inputs outputs name
void Rename(const std::string& old_name, const std::string& new_name);

const VarNameMap& Inputs() const { return inputs_; }
const VarNameMap& Outputs() const { return outputs_; }
//! Get a input with argument's name described in `op_proto`
const std::string& Input(const std::string& name) const;
//! Get a input which has multiple variables.
Expand All @@ -112,10 +114,11 @@ class OperatorBase {

virtual std::vector<std::string> OutputVars(bool has_intermediate) const;

std::string Type() const { return type_; }
const std::string& Type() const { return type_; }
void SetType(const std::string& type) { type_ = type; }
const AttributeMap& Attrs() const { return attrs_; }

public:
protected:
std::string type_;
// NOTE: in case of OpGrad, inputs_ contains:
// I (Inputs)
Expand Down
8 changes: 4 additions & 4 deletions paddle/framework/pybind.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ void ExposeOperator(ClassType &m) {
.def("run", &ClassType::type::Run)
.def("type",
[](const typename ClassType::type &op) -> std::string {
return op.type_;
return op.Type();
})
.def("outputs",
[](const typename ClassType::type &op)
-> std::map<std::string, std::vector<std::string>> {
return op.outputs_;
return op.Outputs();
})
.def("inputs",
[](const typename ClassType::type &op) { return op.inputs_; })
[](const typename ClassType::type &op) { return op.Inputs(); })
.def("__str__", &ClassType::type::DebugString)
.def("no_intermediate_outputs",
[](const typename ClassType::type &op) {
Expand Down Expand Up @@ -229,7 +229,7 @@ All parameter, weight, gradient are variables in Paddle.
net.def_static("create",
[]() -> std::shared_ptr<operators::NetOp> {
auto retv = std::make_shared<operators::NetOp>();
retv->type_ = "plain_net";
retv->SetType("plain_net");
return retv;
})
.def("add_op", &operators::NetOp::AddOp)
Expand Down
4 changes: 2 additions & 2 deletions paddle/operators/net_op.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void NetOp::CompleteAddOp(bool calc) {
std::set<std::string> input_set;
std::set<std::string> output_set;
for (auto& op : ops_) {
for (auto& ipt : op->inputs_) {
for (auto& ipt : op->Inputs()) {
for (auto& var_name : ipt.second) {
if (!Contains(output_set, var_name)) { // Not other op's output
input_set.insert(var_name);
Expand All @@ -39,7 +39,7 @@ void NetOp::CompleteAddOp(bool calc) {
}
}

for (auto& opt : op->outputs_) {
for (auto& opt : op->Outputs()) {
for (auto& var_name : opt.second) {
output_set.insert(var_name);
}
Expand Down
4 changes: 2 additions & 2 deletions paddle/operators/net_op_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ TEST(OpKernel, all) {

net->CompleteAddOp();
AssertSameVectorWithoutOrder({"x", "w1", "b1", "w2", "b2"},
net->inputs_.at(NetOp::kAll));
AssertSameVectorWithoutOrder({"y", "z"}, net->outputs_.at(NetOp::kAll));
net->Inputs(NetOp::kAll));
AssertSameVectorWithoutOrder({"y", "z"}, net->Outputs(NetOp::kAll));

auto final_outs = net->OutputVars(false);

Expand Down
Loading