-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add shape inference design doc #3693
Conversation
This PR summarizes the discussion a few hours ago. Anyone please feel free to complete it. |
paddle/framework/type_inference.md
Outdated
LOD_TENSOR = 6; | ||
} | ||
Type type = 1; | ||
optional LODTesnorDesc lod_tensor = 2; // when type==LOD_TENSOR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOD_TENSOR just a description of tensor's sequence information. It could be float lod tensor,int lod tensor, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We have LODTensor::element_type.
|
||
```protobuf | ||
message VarDesc { | ||
enum Type { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, very strange for Variable Type
. If it is variable, the type should be
- Tensor
- LODTensor
- StepScopes.
- Etc.
Not int/float. They are types for tensors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our Variable is dynamic-typed, like a Python variable, which can hold any typed value. So VarDesc needs a field type
.
A variable can hold a single int, so there should be a enum Type { INT;
.
A tensor can hold any typed elements, so TensorDesc needs a field element_type
.
I am changing the text in accordance.
paddle/framework/type_inference.md
Outdated
} | ||
|
||
message LODTensorDesc { | ||
repeated int dims = 1; // [UNK, UNK, 6000] is saved as [-1, -1, 6000] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
saved as [0, 0, 6000] is better. If unk is the short for batch-size
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 is a valid dim size. Consider that IfOp needs to split the input for its left and right branches. It could be that the input for the left or right branch happens to have 0 instances.
```c++ | ||
class Block : public OperatorBase { | ||
private: | ||
map<string, VarDesc> vars_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shape information is stored in paddle::framework::Tensor
. It is no need to store it again in in VarDesc
.
```c++ | ||
class OperatorBase { | ||
private: | ||
const OpDesc& desc_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are many operators not created by OpDesc
in backward, optimization. It is no need to store OpDesc, and not able to store OpDesc.
InferShape(block); | ||
} | ||
private: | ||
virtual InferShape(Block* block) = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should InferShape
be redesigned again?
Closing this too old PR. |
Variable definition and type inference belong to Block (used to be known as NetOp) instead of Scope.