-
Notifications
You must be signed in to change notification settings - Fork 6
Initial support for Numpy subclasses (for dparray) #116
Conversation
…ve custom allocators/deallocators for them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DrTodd13 I looked at the dparray.py
implementation. I want some minor code quality issues addressed. Also, can you please add a module level docstring with some example?
This PR should be eventually splited into two PRs - one for Numba and another one for numba-dppy. I could take care of spliting this PR to I do not know, maybe there is an intention to move dparray to upstream too. Is it? |
@PokhodenkoSA I am going to open a PR today to move the dparray portion into dpctl. The part that interfaces dparray to Numba will remain in dppy. The Numba changes are in merge_subclass PR. |
Related to #75 and IntelPython/dpctl#184. |
This PR will be refactored in following way:
|
NRT_external_malloc_func malloc; | ||
NRT_external_realloc_func realloc; | ||
NRT_external_free_func free; | ||
void *opaque_data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, the opaque_data
needs to be a function pointer to DPCTLQmgr_GetCurrentQueue
so that the external allocator always uses the current queue from dpctl
for memory allocation. The current implementation stores the queue only once and after that it is never updated, leading to potential issues if the current queue is changed inside dpctl
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spot to mention this would be in dppl_rt.c where we fill in the opaque value. This here is still fine since opaque_data can also store a function pointer.
I will rebase this branch on top of |
Finally this PR should contain changes from numba#6148.
Related work: