Implement the type of data in channel #8285

chengduoZH · 2018-02-08T15:18:09Z

Need to update comment

kavyasrinet

Thanks so much for making this PR! I just have a few questions I have posted.

kavyasrinet · 2018-02-09T00:06:11Z

paddle/framework/framework.proto

+    SELECTED_ROWS = 10;
+    LOD_TENSOR_ARRAY = 11;
+  }
+  required DataType base_type = 1;


From what I understand, base_type would be the channel type (type of elements in channel) right ?

no, the type of element in the channel should be repeated MetaDataDesc.
For example, the type of element is <int32, int64, float32>.

kavyasrinet · 2018-02-09T00:08:59Z

paddle/framework/framework.proto

+  optional bool data_bool = 2;
+  optional int32 data_int32 = 3;
+  optional int64 data_int64 = 4;
+  optional float data_float = 5;


where would these fields be used ? As attributes for MetaDataDesc ?

Yes, these fields are attributes of MetaDataDesc and are optional, so one MetaDataDesc only describes one base_type.

kavyasrinet · 2018-02-09T00:09:55Z

paddle/framework/framework.proto

+  optional LoDTensorArrayDesc tensor_array = 8;
+}
+
+message ChanEleDesc { repeated MetaDataDesc meta_data = 1; }


I assume this step would append multiple elements in the tuple together ? and the next step would append tuples of these ?

We can regard the ChanEleDesc as a tuple, it can describe <int32,int32,int64,float32>.

abhinavarora · 2018-02-08T23:49:01Z

paddle/framework/framework.proto

@@ -118,6 +118,38 @@ message LoDTensorArrayDesc {

 message ReaderDesc { repeated LoDTensorDesc lod_tensor = 1; }

+message MetaDataDesc {
+  enum DataType {
+    BOOL = 1;


Thank you. I see based on the discussion we had on Hi yesterday, you have included the basic data types as well.

abhinavarora · 2018-02-09T00:11:42Z

paddle/framework/framework.proto

+  }
+  required DataType base_type = 1;
+  optional bool data_bool = 2;
+  optional int32 data_int32 = 3;


I am not sure what will we save in these optionals for fundamental types? Could you please explain with an example. For example, if we make a channel which can take tuple of 2 integers, how will this look?

We can define a visitor, and the visitor can get the data of MetaData according to the data type.

abhinavarora · 2018-02-09T00:25:16Z

paddle/framework/framework.proto

+message ChanEleDesc { repeated MetaDataDesc meta_data = 1; }
+
+message ChanDesc {
+  repeated ChanEleDesc channel_type = 1;


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.

I know what you mean, let me think again.

abhinavarora · 2018-02-09T01:24:23Z

paddle/framework/framework.proto

+    TENSOR = 8;
+    LOD_TENSOR = 9;
+    SELECTED_ROWS = 10;
+    LOD_TENSOR_ARRAY = 11;


I am not sure how we will be able to support channel of type channel. Go allows this. Do you think this code would suffice for that? Also, I am not sure if we need it right now.

This is a good question, I think that we should also support channel of type channel, but this code maybe should be changed.

typhoonzero · 2018-02-09T06:27:43Z

paddle/framework/framework.proto

+message ChanEleDesc { repeated VarDesc meta_data = 1; }
+
+message ChanDesc {
+  repeated ChanEleDesc channel_type = 1;


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.

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

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.

wangkuiyi · 2018-02-09T18:35:17Z

paddle/framework/framework.proto

+message ChanDesc {
+  repeated ChanEleDesc channel_type = 1;
+  required int32 cap = 2 [ default = 0 ];
+  required string name = 3;


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.

…feature/add_channel_type

CLAassistant · 2018-02-10T17:33:36Z

All committers have signed the CLA.

chengduoZH · 2018-02-11T01:31:20Z

This PR is not mature enough, and it is turned off first.

chengduoZH requested review from abhinavarora, wangkuiyi, helinwang and kavyasrinet February 8, 2018 15:19

Add Channel type

4ee294d

chengduoZH force-pushed the feature/add_channel_type branch from 6e8144e to 4ee294d Compare February 8, 2018 15:30

kavyasrinet previously approved these changes Feb 8, 2018

View reviewed changes

kavyasrinet reviewed Feb 9, 2018

View reviewed changes

abhinavarora reviewed Feb 9, 2018

View reviewed changes

refine chanDesc

648f949

typhoonzero reviewed Feb 9, 2018

View reviewed changes

wangkuiyi reviewed Feb 9, 2018

View reviewed changes

Update ChannelDesc

1376550

wangkuiyi approved these changes Feb 9, 2018

View reviewed changes

Merge branch 'develop' of http://github.com/paddlepaddle/paddle into …

bbf09d9

…feature/add_channel_type

chengduoZH closed this Feb 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the type of data in channel #8285

Implement the type of data in channel #8285

chengduoZH commented Feb 8, 2018

kavyasrinet left a comment

kavyasrinet Feb 9, 2018

chengduoZH Feb 9, 2018

kavyasrinet Feb 9, 2018

chengduoZH Feb 9, 2018

kavyasrinet Feb 9, 2018 •

edited

Loading

chengduoZH Feb 9, 2018

abhinavarora Feb 8, 2018

abhinavarora Feb 9, 2018

chengduoZH Feb 9, 2018

abhinavarora Feb 9, 2018

chengduoZH Feb 9, 2018

abhinavarora Feb 9, 2018

chengduoZH Feb 9, 2018

typhoonzero Feb 9, 2018

chengduoZH Feb 9, 2018

wangkuiyi Feb 9, 2018

wangkuiyi Feb 9, 2018

CLAassistant commented Feb 10, 2018 •

edited

Loading

chengduoZH commented Feb 11, 2018

Implement the type of data in channel #8285

Implement the type of data in channel #8285

Conversation

chengduoZH commented Feb 8, 2018

kavyasrinet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavyasrinet Feb 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Feb 10, 2018 • edited Loading

chengduoZH commented Feb 11, 2018

kavyasrinet Feb 9, 2018 •

edited

Loading

CLAassistant commented Feb 10, 2018 •

edited

Loading