Closed tqchen closed 7 years ago
I take a stab in making the basic tensor structure here https://github.com/dmlc/dlpack/blob/master/include/dlpack/dlpack.h based on existing discussions
I would like to open a vote discussion for current data structure in dlpack.h, please voice your opinion or simply vote by +1 if you agree. Everyone is welcomed to vote and voice their opinion. If there is no objection in the proposal in one week, we will consider it finalized.
@soumith @edgarriba @bhack @piiswrong @Yangqing
Thanks @tqchen ! It looks great in general, a few minor comments:
What is kCPUPinned? I understand the implication (i.e. cudaMallocHost) but it seems that the implementation won't distinguish kCPU vs kCPUPinned. A bit more documentation would be great.
For DLTypeCodeKind, it might be good to explicitly specify the length, like int8_t, int16_t, int32_t and int64_t. Using int and long has been a headache because compilers have different ways of dealing with them, even templating (see https://github.com/caffe2/caffe2/blob/master/cmake/MiscCheck.cmake#L5-L22)
For DLContext - did you intend to use enum instead of int?
For DLTensor - I like the opaque pointer! This has been something I was very vocal about but not that well accepted by some frameworks.
/cc @naibaf7
@Yangqing One thing that hangs on the DLDataType, is to make it future compatibile with weird types, like int1_t.
But you are right that having such thing makes it a bit hard for templatizing, maybe we can consider use bit packing to pack the code and bits into a single int.
Taking a look to TF data types seems that bool could be useful. What about quantized?
The current data type cane expression quantized data types like int4_t
, by specifying bits=4 and type_code=kInt.
I think what @Yangqing mentioned is being able to use type constant as template argument, using a struct kinda of goes against that. Maybe using a bit packed integer(with same layout as struct) helps to resolve that problem
@Yangqing Consider open a PR with respect your suggested changes?
@tqchen Might be a bit tricky due to my day duties but I'll try at my earliest availability :)
RE templating - I was basically promoting the use of explicit int32_t, int64_t instead of int and long, because that makes cross-platform much easier. I don't think having a struct / enum blocks it and I am not against it :)
Agree with explicit sized types.
Yes. Also if 4bit integer as I remember it is not covered by fixed width integer types right?
@bhack for anything under 8 bit we probably need to be creative.
Normally you do not want to have a array with int4, instead it will be something like int4x32 (a vector type with 32 4bit integer), so operations are vectorized. With the existing type system, it is
type_code=int, bits=4, lanes=32
And for larger registers?
We will handle this with lanes?
The tensor structure does not define the register types, it only defines the flags for the data types. I suppose the operator implementation should deal with vectorization and we know before hand that the vector type can be loaded safely from the storage without worrying about alignment, e.g. opencl have type float4.
Ok it is only flag releated.
/cc @bwasti FYI. What if we have the use case of int5 or int7? :)
(Not saying that we will need those in actual computation unless we have specialized hardware, but might be interesting in serialization if we ever need it.)
The usecase of int5 or int7 could be a good reason to keep bit encoded in type codes, but maybe only defined common enums to let user choose from.
I am going to close the vote in two days, given it is two weeks since the last change and we will view the data structure as pinned.
Ok
sgtm
migrated from https://github.com/dmlc/mxnet/issues/4735