Implement np.take in kernel by dpnp #246
Conversation
| @@ -0,0 +1,63 @@ | |||
| import numba_dppy.dpnp_glue.dpnpimpl as dpnp_ext | |||
There was a problem hiding this comment.
You need to add copyright header
| return c | ||
|
|
||
| test_indices = [] | ||
| test_indices.append(np.array([1, 5, 1, 11, 3])) |
There was a problem hiding this comment.
May be it's better to simplify? Or what the reason of iterating over the list of indexes if you test them in the separate test?
| test_indices.append(np.array([1, 5, 1, 11, 3])) | |
| test_indices = [np.array([1, 5, 1, 11, 3]),] |
| for ind in test_indices: | ||
| self.assertTrue( | ||
| self.check_take_for_different_datatypes( | ||
| f, np.take, self.get_np_array(ind), [12], self.tys, matrix=False |
There was a problem hiding this comment.
I think you can just cast value to np.array here for indexes. Function get_np_array is redundant
There was a problem hiding this comment.
I also think the get_np_array is redundant, what are we trying to test throught this function?
|
@reazulhoque Could you please look into this PR? |
|
|
||
| def test_take_indices(self): | ||
| @njit | ||
| def f(a, ind): |
There was a problem hiding this comment.
Both the f() function seems to perform the same thing, may be 1 common is good enough. Line 1453 and 1466.
| ): | ||
| for ty in tys: | ||
| if matrix: | ||
| a = np.arange(np.prod(np.array(dims)), dtype=ty).reshape( |
There was a problem hiding this comment.
If dims is expected to be a np.array may be let us pass an np.array instead of a list.
| tys = [np.int32, np.uint32, np.int64, np.uint64, np.float, np.double] | ||
|
|
||
| def check_take_for_different_datatypes( | ||
| self, fn, test_fn, ind, dims, tys, matrix=None |
There was a problem hiding this comment.
Since matrix seems to be a boolean, let us assign a boolean as a default value?
|
@vlad-perevezentsev This should be merged with main and also Elena has a PR that adds the same file and other function of same category, we need to be aware of that. |
No description provided.