inducer / pyopencl

OpenCL integration for Python, plus shiny features
http://mathema.tician.de/software/pyopencl
Other
1.04k stars 239 forks source link

Allow None in wait_for lists #626

Closed nrother closed 1 year ago

nrother commented 1 year ago

If None is present in the list of events to wait for, a type error is raised:

kernel_event = None
kernel_event = gbp_kernel(
    queue, output.shape[0:2], None, # queue and grid size
    args... # kernel params
    wait_for=[kernel_event] # events to wait for, kernel_event is None here!
)
TypeError: enqueue_nd_range_kernel(): incompatible function arguments. The following argument types are supported:
    1. (queue: pyopencl._cl.CommandQueue, kernel: pyopencl._cl.Kernel, global_work_size: handle, local_work_size: handle, global_work_offset: handle = None, wait_for: handle = None, g_times_l: bool = False, allow_empty_ndrange: bool = False) -> pyopencl._cl.Event

Invoked with: <pyopencl._cl.CommandQueue object at 0x7fc8752268b0>, <pyopencl._add_functionality.<locals>.kernel_set_arg_types.<locals>.KernelWithCustomEnqueue object at 0x7fc8752a58b0>, (1024, 1024), None, None, [<pyopencl._cl.NannyEvent object at 0x7fc875235770>, <pyopencl._cl.NannyEvent object at 0x7fc875235770>, None], None, False

In this case it is of course simple to just remove the wait_for argument altogether, but the are situations where it might be useful to filter them out inside enqueue_nd_range_kernel. In my case I tried to build a chain of kernels that need to execute sequentially and tried to re-use the same variable for the event. In the first iteration it was None and I hit this problem.

The documentation for clEnqueueNDRangeKernel states that "the list of events pointed to by event_wait_list must be valid", so this would be a derivation from the OpenCL interface, but I think a useful one.

inducer commented 1 year ago

I think I understand where you're coming from, but I don't agree. One main issue is that the enqueue code path is one of PyOpenCL's most cost-sensitive ones, and every convenience feature ultimately comes at a price.

nrother commented 1 year ago

Ok, that makes sense to me. It's also easy to fix in the client code (wait_for=[x for x in wait_list if x is not None]) if one needs it. So let's close this.