Split operator with CPU kernel #4046
Conversation
Yancey1989
changed the title
| @@ -185,10 +185,9 @@ def check_output_with_place(self, place): | |||
| for out_name, out_dup in Operator.get_op_outputs(self.op.type()): | |||
| if out_dup: | |||
| sub_out = self.outputs[out_name] | |||
| for sub_out_name in sub_out: | |||
| for sub_out_name, expect in sub_out: | |||
There was a problem hiding this comment.
for sub_out_name, expect in sub_out.iteritems():
There was a problem hiding this comment.
sub_out a tuple list, so maybe no iteritems.
There was a problem hiding this comment.
You can add some type checking code, so that if something went wrong, the python traceback message will be more readable
| self.op_type = "split" | ||
| axis = 0 | ||
| indices = 2 | ||
| sections = [1, 3] |
There was a problem hiding this comment.
Seems sections not used.
| @@ -185,10 +185,9 @@ def check_output_with_place(self, place): | |||
| for out_name, out_dup in Operator.get_op_outputs(self.op.type()): | |||
| if out_dup: | |||
| sub_out = self.outputs[out_name] | |||
| for sub_out_name in sub_out: | |||
| for sub_out_name, expect in sub_out: | |||
There was a problem hiding this comment.
You can add some type checking code, so that if something went wrong, the python traceback message will be more readable
paddle/operators/split_op.cc
Outdated
| size_t num = static_cast<size_t>(ctx.Attr<int>("num")); | ||
| std::vector<int> sections = | ||
| static_cast<std::vector<int>>(ctx.Attr<std::vector<int>>("sections")); | ||
| size_t n = outs.size(); |
| AddInput("X", "the input tensor of split operator."); | ||
| AddOutput("Out", "the output tensors of split operator.").AsDuplicable(); | ||
| AddComment(R"DOC( | ||
| Split the input tensor into multiple sub-tensors. |
There was a problem hiding this comment.
Should also have some descriptions about attributes "num".
paddle/operators/split_op.h
Outdated
| auto outs = ctx.MultiOutput<framework::Tensor>("Out"); | ||
| int64_t axis = static_cast<int64_t>(ctx.Attr<int>("axis")); | ||
| size_t before = 1, after = 1; | ||
| size_t n = outs.size(); |
paddle/operators/split_op.h
Outdated
| size_t n = outs.size(); | ||
| size_t input_axis_dim = 0; | ||
| for (size_t i = 0; i < n; i++) { | ||
| input_axis_dim += outs[i]->dims()[axis]; |
There was a problem hiding this comment.
Why not use in->dims()[axis]?
There was a problem hiding this comment.
Sure! It's my mistake.
| : NetOp(type, inputs, outputs, attrs) { | ||
| auto out_grad = Inputs(framework::GradVarName("Out")); | ||
| auto x_grad = Output(framework::GradVarName("X")); | ||
| AppendOp(framework::OpRegistry::CreateOp("concat", {{"X", out_grad}}, |
There was a problem hiding this comment.
Can we call the kernel directly instead of using netOP?
There was a problem hiding this comment.
Maybe it's OK, but it's better we have a common solution in #4099 , and I will fix this in another PR.
Releated issue #3929