cornell-zhang/heterocl

hcl.Struct doesn't support dtype specified as strings (e.g., 'uint16')

jcasas00 opened this issue · 7 comments

Code:

hcl.placeholder((), "A", hcl.UInt(16))
hcl.placeholder((), "A", 'uint16')

hcl.Struct ({'foo': hcl.UInt(16) })
hcl.Struct ({'foo': 'uint16' })

Output:

>>> hcl.placeholder((), "A", hcl.UInt(16))                          # OK
<heterocl.tensor.Scalar object at 0x7f89f723f670>
>>> hcl.placeholder((), "A", 'uint16')                                # OK
<heterocl.tensor.Scalar object at 0x7f89f7155a60>
>>>
>>> hcl.Struct ({'foo': hcl.UInt(16) })                                 # OK
Struct(OrderedDict([('foo', UInt(16))]))
>>> hcl.Struct ({'foo': 'uint16' })                                        # not OK
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/heterocl/python/heterocl/types.py", line 65, in __init__
    self.bits += dtype.bits
AttributeError: 'str' object has no attribute 'bits'

hcl.Struct should support both forms of dtype specifications as other APIs do (e.g., hcl.placeholder [as above], hcl.scalar, etc.) for consistency.

Similar issue occurs in TensorSlice class getattr and setattr methods.

Similar issue occurs in TensorSlice class getattr and setattr methods.

These two methods should be fine? Since we are accessing with keys (i.e., the name field).

After I temporarily patched hcl.Struct (replacing usage of dtype.bits with get_bitwidth(dtype)) , it worked with the test case I showed above
But my bigger program hit the same problem in the TensorSlice class where I had to apply the same change in the following code in both getattr and setattr:
for dkey, dval in hcl_dtype.dtype_dict.items():
if dkey == key:
#end = start + dval.bits # old code
end = start + types.get_bitwidth(dval) # new code
dtype = types.dtype_to_str(dval)
break
else:
#start += dval.bits # old code
start += types.get_bitwidth(dval) # new code

I see, but my fix would not generate such a problem. Will let you know when it's done.

That would be a better fix then. Thanks.

Please check #411.

Tested latest master and works for me now. Thanks.