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

Assign Operator. #5531

Merged
merged 3 commits into from
Nov 14, 2017
Merged

Assign Operator. #5531

merged 3 commits into from
Nov 14, 2017

Conversation

reyoung
Copy link
Collaborator

@reyoung reyoung commented Nov 10, 2017

Out=X, when type in [LoDTensor/SelectedRows/LoDTensorArray]


void operator()(const framework::SelectedRows &rows) const {
auto &out_rows = *out_->GetMutable<framework::SelectedRows>();
*out_rows.mutable_rows() = rows.rows();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

auto &out_rows = *out_->GetMutable<framework::SelectedRows>();
*out_rows.mutable_rows() = rows.rows();
auto &t = rows.value();
out_rows.mutable_value()->CopyFrom(t, t.place(), dev_ctx_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return;
}
auto *out = scope.FindVar(Output("Out"));
PADDLE_ENFORCE(out != nullptr, "Must set output if x is set");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must set output if x is set ->

The Output(Out) should not be null if the Input(X) is set.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

: OpProtoAndCheckerMaker(proto, op_checker) {
AddInput(
"X",
"The input variable. Could be LoDTensor/SelectedRows/LoDTensorArray.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

" (LoDTensor, SelectedRows or LoDTensorArray) The input variable could be LoDTensor, SelectedRows or LoDTensorArray. "

The most operators' doc follow format: https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/operators/gru_op.cc#L73

() specify the type, then explain it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

"X",
"The input variable. Could be LoDTensor/SelectedRows/LoDTensorArray.")
.AsDispensable();
AddOutput("Out", "The output variable. Its type will be set as same as X");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

" (LoDTensor, SelectedRows or LoDTensorArray) The type of output is the same as input X."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

AddOutput("Out", "The output variable. Its type will be set as same as X");
AddComment(R"DOC(Assign Operator

Out = X, when type in [LoDTensor/SelectedRows/LoDTensorArray]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tell the user how to process other types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

class TestAccuracyOp(op_test.OpTest):
def setUp(self):
self.op_type = "assign"
x = numpy.random.random(size=(100, 10))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, the input type is LoDTensor. Whether needs to add the unit test to others types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other types of variable are not exposed to Python well. It will be tested during if-else operator's unit tests.

It will be added in the following PRs

import unittest


class TestAccuracyOp(op_test.OpTest):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestAccuracyOp --> TestAssignOp

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


void operator()(const framework::SelectedRows &rows) const {
auto &out_rows = *out_->GetMutable<framework::SelectedRows>();
*out_rows.mutable_rows() = rows.rows();
Copy link
Member

@QiJune QiJune Nov 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need to

out_rows.set_height(rows.height());

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

@QiJune QiJune left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@reyoung reyoung merged commit 7c1755d into PaddlePaddle:develop Nov 14, 2017
@reyoung reyoung deleted the feature/assign_op branch December 26, 2017 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants