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.

Sign up for GitHub

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

Jump to bottom

Implement the type of data in channel #8285

Closed
wants to merge 4 commits into from
Closed

Implement the type of data in channel #8285

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ee294d
Add Channel type
chengduoZH Feb 8, 2018
648f949
refine chanDesc
chengduoZH Feb 9, 2018
1376550
Update ChannelDesc
wangkuiyi Feb 9, 2018
bbf09d9
Merge branch 'develop' of http://github.com/paddlepaddle/paddle into …
wangkuiyi Feb 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions paddle/framework/framework.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ message LoDTensorArrayDesc {

message ReaderDesc { repeated LoDTensorDesc lod_tensor = 1; }

message ChanEleDesc { repeated VarDesc meta_data = 1; }

message ChanDesc {
repeated ChanEleDesc channel_type = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also explain why ChanEleDesc is repeated here? From what I understand, the types of each of the dimensions in the n-tuple is captured by the statement repeated MetaDataDesc meta_data. Are we trying to make a tuple of tuples here? Sorry about the confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know what you mean, let me think again.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think ChanEleDesc maybe not needed, each element inside one channel instance should be of the same type. So the ChanDesc should look like:

message ChanDesc {
required VarType ele_type = 1;
required int32 cap = 2 [ default = 0 ];
required string name = 3;
}

Then VarType should have a new type called VAR_LIST representing a list of variables, so that element like tensor, label is able to put inside the channel.

The contents of the channel are manipulated at runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@typhoonzero thanks for your review! And this is a good suggestion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think all the work to do in this PR is to add the following definition of ChannelDesc:

message ChannelDesc {
  VarDesc elem_type = 1;
  int cap = 2;
}

Nothing more than that.

required int32 cap = 2 [ default = 0 ];
required string name = 3;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand why would we need name here. I can see that we do need cap.

A reference design is the Go's channel definition: https://github.com/golang/go/blob/4fc9565ffce91c4299903f7c17a275f0786734a1/src/runtime/chan.go#L17-L29

We are particularly interested in elemtype.

}

message VarDesc {
enum VarType {
LOD_TENSOR = 1;
Expand All @@ -129,6 +137,7 @@ message VarDesc {
LOD_TENSOR_ARRAY = 7;
PLACE_LIST = 8;
READER = 9;
CHAN = 10;
}
required string name = 1;
required VarType type = 2;
Expand All @@ -137,6 +146,7 @@ message VarDesc {
optional TensorDesc selected_rows = 5;
optional LoDTensorArrayDesc tensor_array = 6;
optional ReaderDesc reader = 7;
optional ChanDesc chan = 8;
}

message BlockDesc {
Expand Down