Fixing typos, grammatical and spelling mistakes in Refactorization Overview document #4479

abhinavarora · 2017-09-28T19:04:19Z

No description provided.

dzhwinter

LGTM!
A great job which enhanced our refactoring document!

NorthStar · 2017-09-28T21:43:16Z

doc/design/refactorization.md

 1. A graph is composed of *variables* and *operators*.

-1. The description of graphs must be able to be serialized/deserialized, so it
+1. The description of graphs must be capable of being serialized/deserialized, so that


must be serializable

NorthStar · 2017-09-28T21:45:23Z

doc/design/refactorization.md

+1. The Python program does the following steps

-   1. *compilation*: runs a Python program to generate a protobuf message representation of the graph and send it to
+   1. *compilation*: run a Python program to generate a protobuf message representation of the graph and send it to


Compilation runs

NorthStar · 2017-09-28T21:45:45Z

doc/design/refactorization.md

+   1. It can be sent to clients for mobile or enterprise deployment.

-1. The Python program do
+1. The Python program does the following steps


The Python program does two things:

NorthStar · 2017-09-28T21:46:43Z

doc/design/refactorization.md

      1. the master process of a distributed training job for training, or
      1. the server process of a Kubernetes serving job for distributed serving.
-   1. *execution*: according to the protobuf message, constructs instances of class `Variable` and `OperatorBase`, and run them.
+   1. *execution*: execute the graph by constructing instances of class [`Variable`](https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/framework/variable.h#L24) and [`OperatorBase`](https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/framework/operator.h#L70), according to the protobuf message.


*Execution* executes the graph

NorthStar · 2017-09-28T21:47:16Z

doc/design/refactorization.md

+## Description and Realization of Computation Graph

-At compile time, the Python program generates protobuf message representation of the graph, or the description of the graph.
+At compile time, the Python program generates a protobuf message representation of the graph, or the description of the graph.


a description of the graph.

NorthStar · 2017-09-28T22:03:59Z

doc/design/refactorization.md

+1. Write an Op class and its gradient Op class, if required.
+2. Write an Op maker class. In the constructor of this class, describe the inputs, outputs and attributes of the operator.
+3. Invoke the macro `REGISTER_OP`. This macro will
+	1. Call maker class to complete the `proto` and the `checker`


Drop the articles here.

NorthStar · 2017-09-28T22:04:46Z

doc/design/refactorization.md

+2. Write an Op maker class. In the constructor of this class, describe the inputs, outputs and attributes of the operator.
+3. Invoke the macro `REGISTER_OP`. This macro will
+	1. Call maker class to complete the `proto` and the `checker`
+	2. Using the completed `proto` and `checker`, it will add a new key-value pair to the `OpInfoMap`


Add a new key-value pair to OpInfoMap using the completed proto and checker.

NorthStar · 2017-09-28T22:05:37Z

doc/design/refactorization.md

+	2. Using the completed `proto` and `checker`, it will add a new key-value pair to the `OpInfoMap`

-4. Invoke `USE` macro in where the Op is used to make sure it is linked.
+4. Invoke the `USE` macro in which the Op is used, to make sure that it is linked.


Drop the comma here.

Invoke the USE macro where an Op is used to make sure that it is linked.

NorthStar · 2017-09-28T22:07:01Z

doc/design/refactorization.md

-	- shared variable => insert `Add` operator
-	- no gradient => insert `fill_zero_grad` operator
-	- recursive netOp => call `Backward` recursively
+- **Input**: graph of forwarding operators


NorthStar · 2017-09-28T22:09:55Z

doc/design/refactorization.md

-	* `Scope` has a hierarchical structure. The local scope can get variable from its parent scope.
+	* All operations on `Tensor` are written in `Operator` or global functions.
+	* Variable length Tensor design [LoDTensor](https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/framework/lod_tensor.md)
+* `Variable` instances are the inputs and the outputs of an operator. Not just `Tensor`.


, not just Tensor.

pengli09

One critical error and a few potential typos.

pengli09 · 2017-09-29T02:11:51Z

doc/design/refactorization.md


-   1. could to be sent to the cloud for distributed execution, and
-   1. be sent to clients for mobile or enterprise deployment.
+   1. It can to be sent to the cloud for distributed execution, and


s/can to be/can be/g

pengli09 · 2017-09-29T02:22:27Z

doc/design/refactorization.md

+   1. Infer the type and the shape of variables,
+   1. Plan memory-reuse for variables,
+   1. Generate the backward graph
+   1. Optimize the computation graph.


Here "optimization'' refers to the optimization operators related stuff such as Adam, which is also a part of the graph. Therefore, "Optimize the computation graph." is incorrect.

pengli09 · 2017-09-29T02:34:38Z

doc/design/refactorization.md

-    * Use `Run` to compute `input variables` to `output variables`.
+* `Operator` is the fundamental building block of the user interface.
+    * Operator stores input/output variable names, and attributes.
+    * The `InferShape` interface is used to infer the shape of the output variable shapes based on the shapes of the input variables.


s/the output variable shapes/the output variables/g

pengli09 · 2017-09-29T02:46:54Z

doc/design/refactorization.md

+	* Variable length Tensor design [LoDTensor](https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/framework/lod_tensor.md)
+* `Variable` instances are the inputs and the outputs of an operator. Not just `Tensor`.
+	* `step_scopes` in RNN is a variable and not a tensor.
+* `Scope` is where variables are stores.


pengli09 · 2017-09-29T02:48:03Z

doc/design/refactorization.md

+* `Variable` instances are the inputs and the outputs of an operator. Not just `Tensor`.
+	* `step_scopes` in RNN is a variable and not a tensor.
+* `Scope` is where variables are stores.
+	* map<string `variable_name`, Variable>


I think /* var name*/ is better.

pengli09 · 2017-09-29T02:50:52Z

doc/design/refactorization.md

-  - take the documentation quality into account when doing PR
-  - preview the documentations, read and improve them from users' perspective
+- Compare the performance of migrated models with old ones.
+- Follow the google C++ style


s/style/style./g
Note the period.

Fixing typos and grammatical mistakes in Refactorization documents

536cda0

abhinavarora self-assigned this Sep 28, 2017

abhinavarora requested review from NorthStar, dzhwinter and pengli09 September 28, 2017 19:04

dzhwinter approved these changes Sep 28, 2017

View reviewed changes

abhinavarora merged commit 5d6d2bc into PaddlePaddle:develop Sep 28, 2017

NorthStar reviewed Sep 28, 2017

View reviewed changes

pengli09 reviewed Sep 29, 2017

View reviewed changes

abhinavarora mentioned this pull request Oct 10, 2017

Fixing errors in the refactorization doc #4680

Merged

Fixing typos, grammatical and spelling mistakes in Refactorization Overview document #4479

Fixing typos, grammatical and spelling mistakes in Refactorization Overview document #4479

Uh oh!

Conversation

abhinavarora commented Sep 28, 2017

Uh oh!

dzhwinter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pengli09 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants