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

Refine the doc for mul_op and fully connected layer. #6792

Merged
merged 5 commits into from
Dec 22, 2017

Conversation

lcy-seso
Copy link
Contributor

@lcy-seso lcy-seso commented Dec 20, 2017

fixes #6785 #6835

Copy link
Contributor

@abhinavarora abhinavarora left a comment

Choose a reason for hiding this comment

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

Minor grammatical changes. Make sure to follow the comments syntax in the naming convention for inputs and outputs.

AddInput("X", "The first input of mul op");
AddInput("Y", "The second input of mul op");
AddOutput("Out", "The output of mul op");
AddInput("X", "The first input tensor of the mul op.");
Copy link
Contributor

Choose a reason for hiding this comment

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

This should The first input tensor of mul op. There should not be a the

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

AddInput("Y", "The second input of mul op");
AddOutput("Out", "The output of mul op");
AddInput("X", "The first input tensor of the mul op.");
AddInput("Y", "The second input tensor of the mul op.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor 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 of mul op");
AddInput("X", "The first input tensor of the mul op.");
AddInput("Y", "The second input tensor of the mul op.");
AddOutput("Out", "The output tensor of the mul op.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -73,30 +73,42 @@ class MulOpMaker : public framework::OpProtoAndCheckerMaker {
public:
MulOpMaker(OpProto* proto, OpAttrChecker* op_checker)
: OpProtoAndCheckerMaker(proto, op_checker) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also be following the comment syntax as given in the naming convention -> https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/operators/name_convention.md

Copy link
Contributor Author

@lcy-seso lcy-seso Dec 21, 2017

Choose a reason for hiding this comment

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

I check the name convention in this doc https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/operators/op_documentation/name_convention.md, the current input names and the output name do not violate our name convention. So I only fix the comment style suggested in this doc. If I miss still something, please point it out. Thank you.

flattened to form the first dimension of the final matrix (height
of the matrix), and the rest `rank(X) - num_col_dims` dimensions
are flattened to form the second dimension of the final matrix (
width of the matrix). As a result, height of the flattened matrix
Copy link
Contributor

Choose a reason for hiding this comment

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

height -> the height

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

are flattened to form the second dimension of the final matrix (
width of the matrix). As a result, height of the flattened matrix
is equal to the product of `X`'s first `x_num_col_dims` dimensions'
sizes, and width of the flattened matrix is equal to the product
Copy link
Contributor

Choose a reason for hiding this comment

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

width -> the width

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

sizes, and width of the flattened matrix is equal to the product
of `X`'s last `rank(x) - num_col_dims` dimensions' size.
For example, suppose `X` is a 6-dimensional tensor with the shape
[2, 3, 4, 5, 6], and `x_num_col_dims` = 3. Then, the flattened
Copy link
Contributor

Choose a reason for hiding this comment

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

Then -> Thus

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

in that case, tensors will be reshaped to a matrix. Just like input `X`.
R"DOC(The mul_op can take tensors with more than two dimensions as its
inputs. If the input `Y` is a tensor with more than two
dimensions, `Y` will be flatten into a two-dimensional matrix
Copy link
Contributor

Choose a reason for hiding this comment

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

flatten -> flattened

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

tensor will first be flattened into a 2-dimensional
matrix. The parameter `num_flatten_dims` determines
how the input tensor is flattened: the first
`num_flatten_dims` dimensions will be flatten to form
Copy link
Contributor

Choose a reason for hiding this comment

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

flatten -> flattened

Copy link
Contributor Author

@lcy-seso lcy-seso left a comment

Choose a reason for hiding this comment

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

follow comments.

AddInput("X", "The first input of mul op");
AddInput("Y", "The second input of mul op");
AddOutput("Out", "The output of mul op");
AddInput("X", "The first input tensor of the mul op.");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

AddInput("Y", "The second input of mul op");
AddOutput("Out", "The output of mul op");
AddInput("X", "The first input tensor of the mul op.");
AddInput("Y", "The second input tensor of the mul op.");
Copy link
Contributor 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 of mul op");
AddInput("X", "The first input tensor of the mul op.");
AddInput("Y", "The second input tensor of the mul op.");
AddOutput("Out", "The output tensor of the mul op.");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

flattened to form the first dimension of the final matrix (height
of the matrix), and the rest `rank(X) - num_col_dims` dimensions
are flattened to form the second dimension of the final matrix (
width of the matrix). As a result, height of the flattened matrix
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

are flattened to form the second dimension of the final matrix (
width of the matrix). As a result, height of the flattened matrix
is equal to the product of `X`'s first `x_num_col_dims` dimensions'
sizes, and width of the flattened matrix is equal to the product
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

in that case, tensors will be reshaped to a matrix. Just like input `X`.
R"DOC(The mul_op can take tensors with more than two dimensions as its
inputs. If the input `Y` is a tensor with more than two
dimensions, `Y` will be flatten into a two-dimensional matrix
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

sizes, and width of the flattened matrix is equal to the product
of `X`'s last `rank(x) - num_col_dims` dimensions' size.
For example, suppose `X` is a 6-dimensional tensor with the shape
[2, 3, 4, 5, 6], and `x_num_col_dims` = 3. Then, the flattened
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -73,30 +73,42 @@ class MulOpMaker : public framework::OpProtoAndCheckerMaker {
public:
MulOpMaker(OpProto* proto, OpAttrChecker* op_checker)
: OpProtoAndCheckerMaker(proto, op_checker) {
Copy link
Contributor Author

@lcy-seso lcy-seso Dec 21, 2017

Choose a reason for hiding this comment

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

I check the name convention in this doc https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/operators/op_documentation/name_convention.md, the current input names and the output name do not violate our name convention. So I only fix the comment style suggested in this doc. If I miss still something, please point it out. Thank you.

@abhinavarora
Copy link
Contributor

LGTM!

@lcy-seso lcy-seso merged commit 298dc89 into PaddlePaddle:develop Dec 22, 2017
@lcy-seso lcy-seso deleted the refine_doc branch December 23, 2017 09:30
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.

A mistake in the comment of mul_op
2 participants