dmlc / dlpack

common in-memory tensor structure
https://dmlc.github.io/dlpack/latest
Apache License 2.0
911 stars 133 forks source link

Signedness of attributes like `DLTensor::ndim`, `DLTensor::shape` #102

Closed wjakob closed 1 year ago

wjakob commented 2 years ago

Dear DLpack authors,

I was curious why several definitions in dlpack.h, specifically various DLTensor attributes are signed, when negative-valued arguments would seem to indicate obviously nonsensical tensor configurations (such as negative dimensions or a negative shape along a dimension).

Would PR to change these to an unsigned counterpart be accepted? ABI-wise, there should be no impact as they occupy the same amount of memory (and values using the sign bit would, in any case, not correspond to valid configurations).

Thanks, Wenzel

tqchen commented 2 years ago

Thanks @wjakob! I agree it might be worthwhile to document the choice.

There are different arguments around using signed integers vs unsigned integers (search over the web and there are quite a few discussions around them).

Each side had some valid arguments, in short:

When making the data structure choice initially we felt that making things signed out-weights the value of additional 1bit. I believe this was also used by a few frameworks as well(e.g. PyTorch, TVM).

wjakob commented 2 years ago

Okay, that makes sense-- thanks for clarifying!