Conversation
|
Is this going in the direction you had in mind @inducer ? |
a906d51 to
09553cf
Compare
| class TransferMapper(CopyMapper): | ||
| def __init__(self, to_device: bool, queue: Any, allocator: Any = None) -> None: | ||
| super().__init__() | ||
| self.to_device = to_device |
There was a problem hiding this comment.
I think I would prefer two different mappers, one per direction.
|
|
||
| # {{{ TransferMapper | ||
|
|
||
| class TransferMapper(CopyMapper): |
There was a problem hiding this comment.
I think this guy/these guys would be better off in arraycontext.
There was a problem hiding this comment.
Would the idea be to use to_numpy/from_numpy instead of .data.get then?
|
|
||
| from pyopencl.array import Array as CLArray, to_device | ||
|
|
||
| if isinstance(expr.data, CLArray) and not self.to_device: |
There was a problem hiding this comment.
I think the to-host version of this should error if a non-numpy array is encountered.
There was a problem hiding this comment.
Wouldn't it be worthwhile to support a scenario such as transfer_to_host(transfer_to_host(foo_dag))?
There was a problem hiding this comment.
🤔 Not sure.
Off the bat: maybe not?
There was a problem hiding this comment.
removed support for this scenario in inducer/arraycontext#282
|
|
||
| # {{{ TransferMapper | ||
|
|
||
| class TransferMapper(CopyMapper): |
There was a problem hiding this comment.
Would the idea be to use to_numpy/from_numpy instead of .data.get then?
|
|
||
| from pyopencl.array import Array as CLArray, to_device | ||
|
|
||
| if isinstance(expr.data, CLArray) and not self.to_device: |
| return DataWrapper( | ||
| data=data, | ||
| shape=expr.shape, | ||
| axes=expr.axes, | ||
| tags=expr.tags, | ||
| non_equality_tags=expr.non_equality_tags) |
There was a problem hiding this comment.
Can we make this return an instance of a new DataWrapper type that can __hash__ by data instead of id? That would remove the need for data wrapper deduplication, if I understand the intended use of these mappers correctly.
|
Closing here, will be continued in inducer/arraycontext#282 |
Implements part of #547. Continued in inducer/arraycontext#282.
TODOs: