ledatelescope / bifrost

A stream processing framework for high-throughput applications.
BSD 3-Clause "New" or "Revised" License
66 stars 29 forks source link

bf.map does not work on ci32 type #130

Closed telegraphic closed 2 years ago

telegraphic commented 5 years ago

The complex int32 type doesn't work with the map function at the moment:

a = bf.ndarray(np.array([1,2,3,4,5]), space='cuda', dtype='ci32')
b = bf.ndarray(np.array([1,2,3,4,5]), space='cuda', dtype='ci32')
bf.map("a = b", data={'a': a, 'b': b})

This raises a runtime error:

RuntimeError                              Traceback (most recent call last)
<ipython-input-38-faef62e8891f> in <module>()
----> 1 bf.map("a = b", data={'a': a, 'b': b})

/home/dprice/python/lib/python2.7/site-packages/bifrost-0.8.0-py2.7.egg/bifrost/map.pyc in map(func_string, data, axis_names, shape, func_name, extra_code, block_shape, block_axes)
    124                      narg, _array(args), _array(arg_names),
    125                      func_name, func_string, extra_code,
--> 126                      _array(block_shape), _array(block_axes)))

/home/dprice/python/lib/python2.7/site-packages/bifrost-0.8.0-py2.7.egg/bifrost/libbifrost.pyc in _check(status)
    101             else:
    102                 status_str = _bf.bfGetStatusString(status)
--> 103                 raise RuntimeError(status_str)
    104     else:
    105         if status == _bf.BF_STATUS_END_OF_DATA:

It looks like the dtype2ctype_string hasn't been implemented https://github.com/ledatelescope/bifrost/blob/8e1c0e13f8ba916ee1fc271943f7a2d0a836e849/src/array_utils.hpp#L63 (it's commented out).

Int32 is probably more useful now than it was previously, as the dp4a instruction takes 8-bit inputs and accumulates into a 32-bit integer. I'll see if I can get things working, but any thoughts / comments about why this was tricky / left as a todo would be appreciated.

jaycedowell commented 5 years ago

The only thing I can think of is that there it could be related to the underlying Complex class. If you look in Complex.hpp, Complex<int> isn't really addressed by the current structure. It's not a floating point type but it also isn't a storage type.

jaycedowell commented 5 years ago

While I'm at it this also reminds me of the off-and-on problems I hit with map and mixing various complex types. It usually involves trying to work with storage types and them not getting automatically promoted when they are involved in some math.

jaycedowell commented 2 years ago

What all do you want to do with ci32? There are two ways to go: a pure storage type and something you can do arithmetic on.

jaycedowell commented 2 years ago

https://github.com/ledatelescope/bifrost/commit/cb0c7a23f3fb28c6611ed8346b82e7b01113453e adds support for ci32 as a storage formation, i.e., you can work with it but it has to be converted to Complex<float> to do any math.

telegraphic commented 2 years ago

This seems reasonable to me. Assuming it's the same for ci4 / ci8 / ci16?

jaycedowell commented 2 years ago

Yes, that's the plan. I'm also trying to decide if it is worth the effort to build all of the coercion rules in for the Complex storage types. For example, right now Complex<float>*Complex<int> would result in an error in a bf.map call.

jaycedowell commented 2 years ago

Closing with the release of v0.10.0.