Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bf.map does not work on ci32 type #130

Closed
telegraphic opened this issue Aug 12, 2019 · 7 comments
Closed

bf.map does not work on ci32 type #130

telegraphic opened this issue Aug 12, 2019 · 7 comments
Labels

Comments

@telegraphic
Copy link
Collaborator

telegraphic commented Aug 12, 2019

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
Copy link
Member

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
Copy link
Member

jaycedowell commented Aug 13, 2019

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
Copy link
Member

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
Copy link
Member

cb0c7a2 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
Copy link
Collaborator Author

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

@jaycedowell
Copy link
Member

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
Copy link
Member

Closing with the release of v0.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants