pyopencl icon indicating copy to clipboard operation
pyopencl copied to clipboard

An error in the documentation of cl.enqueue_copy

Open doetools opened this issue 4 years ago • 2 comments

In this documentation, I think it should be dst_offset instead of dest_offset, per the input arguments of function _enqueue_copy_buffer in pyopencl/cffi_cl.py, on line 1330.

image

doetools avatar Jun 10 '21 15:06 doetools

It turned out I was confused by cl.enqueue_copy and _cl._enqueue_copy_buffer . The input argument of the first uses order of dst_buffer, src_buffer while that of the second uses src_buffer, dst_buffer (I guess the reason is to make it be on par with OpenCL specification of clEnqueueCopyBuffer). The former one uses dest_offset while the latter uses dst_offset as positional argument. In the _init_.py, there is a conversion to pop out dest_offset key and writes a key of dst_offset.

Don't get me wrong, I love using pyopencl (even more than C OpenCL)! It was my feeling that this part took me some effort to figure out. Someone with a C OpenCL background (like me) might come across this situation too.

doetools avatar Jun 10 '21 16:06 doetools

  • eneuque_copy unifies all the copy interfaces, and it adopts C memcpy's (target, source) ordering consistently. That's admittedly different from clEnqueueCopyBuffer, but at this point I'd say that's unlikely to change.
  • I agree that the dest_offset vs dst_offset kwarg naming is a bit unfortunate and inconsistent. I'd be happy to take a patch that deprecates (but still supports!) the old argument name while introducing (and documenting) the new one.

inducer avatar Jun 10 '21 21:06 inducer

#452 has deprecations to standardize on dst_offset.

inducer avatar Sep 16 '22 23:09 inducer