-
Notifications
You must be signed in to change notification settings - Fork 16
add a CL device<->host transfer mapper #548
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1951,4 +1951,52 @@ def cached_data_wrapper_if_present(ary: ArrayOrNames) -> ArrayOrNames: | |
|
|
||
| # }}} | ||
|
|
||
|
|
||
| # {{{ TransferMapper | ||
|
|
||
| class TransferMapper(CopyMapper): | ||
| def __init__(self, to_device: bool, queue: Any, allocator: Any = None) -> None: | ||
| super().__init__() | ||
| self.to_device = to_device | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I would prefer two different mappers, one per direction.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in inducer/arraycontext#282 |
||
| self.queue = queue | ||
| self.allocator = allocator | ||
|
|
||
| def map_data_wrapper(self, expr: DataWrapper) -> Array: | ||
| import sys | ||
| if "pyopencl" not in sys.modules: | ||
| return super().map_data_wrapper(expr) | ||
|
|
||
| from pyopencl.array import Array as CLArray, to_device | ||
|
|
||
| if isinstance(expr.data, CLArray) and not self.to_device: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the to-host version of this should error if a non-numpy array is encountered.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-numpy or non-CL?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it be worthwhile to support a scenario such as
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 Not sure. Off the bat: maybe not?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed support for this scenario in inducer/arraycontext#282 |
||
| data = expr.data.get() | ||
| return DataWrapper( | ||
| data=data, | ||
| shape=expr.shape, | ||
| axes=expr.axes, | ||
| tags=expr.tags, | ||
| non_equality_tags=expr.non_equality_tags) | ||
|
Comment on lines
+1973
to
+1978
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make this return an instance of a new |
||
| elif isinstance(expr.data, np.ndarray) and self.to_device: | ||
| data = to_device(self.queue, expr.data, allocator=self.allocator) | ||
| return DataWrapper( | ||
| data=data, | ||
| shape=expr.shape, | ||
| axes=expr.axes, | ||
| tags=expr.tags, | ||
| non_equality_tags=expr.non_equality_tags) | ||
|
|
||
| return super().map_data_wrapper(expr) | ||
|
|
||
|
|
||
| def transfer_to_device(expr: ArrayOrNames, queue: Any, | ||
| allocator: Any = None) -> ArrayOrNames: | ||
| return TransferMapper(True, queue, allocator)(expr) | ||
|
|
||
|
|
||
| def transfer_to_host(expr: ArrayOrNames, queue: Any, | ||
| allocator: Any = None) -> ArrayOrNames: | ||
| return TransferMapper(False, queue, allocator)(expr) | ||
|
|
||
| # }}} | ||
|
|
||
| # vim: foldmethod=marker | ||
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.
I think this guy/these guys would be better off in arraycontext.
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.
Would the idea be to use
to_numpy/from_numpyinstead of.data.getthen?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.
Yeah.
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.
Done in inducer/arraycontext#282