Dealing with non-contiguous memoryviews as binary buffers

[martinal]

While testing the latest ipywidgets support for passing numpy arrays as binary buffers with metadata, I inadvertedly tried to assign a numpy slice to a traittypes.Array trait, and this custom function (lifted from pythreejs) then creates a memoryview of a non-contiguous ndarray:

def array_to_json(value, widget):
    return {
        'shape': value.shape,
        'dtype': str(value.dtype),
        'buffer': memoryview(value)
    }

which shortly after fails inside zmq:

ERROR:tornado.general:Uncaught exception, closing connection.
Traceback (most recent call last):
  File "/home/martinal/envs/visenv-dev/lib/python3.5/site-packages/zmq/eventloop/zmqstream.py", line 414, in _run_callback
    callback(*args, **kwargs)
  File "/home/martinal/envs/visenv-dev/lib/python3.5/site-packages/tornado/stack_context.py", line 277, in null_wrapper
    return fn(*args, **kwargs)
  File "/home/martinal/envs/visenv-dev/lib/python3.5/site-packages/ipykernel/iostream.py", line 105, in _handle_event
    event_f()
  File "/home/martinal/envs/visenv-dev/lib/python3.5/site-packages/ipykernel/iostream.py", line 199, in <lambda>
    self.schedule(lambda : self._really_send(*args, **kwargs))
  File "/home/martinal/envs/visenv-dev/lib/python3.5/site-packages/ipykernel/iostream.py", line 207, in _really_send
    self.socket.send_multipart(msg, *args, **kwargs)
  File "/home/martinal/envs/visenv-dev/lib/python3.5/site-packages/zmq/sugar/socket.py", line 366, in send_multipart
    self.send(msg, SNDMORE|flags, copy=copy, track=track)
  File "zmq/backend/cython/socket.pyx", line 636, in zmq.backend.cython.socket.Socket.send (zmq/backend/cython/socket.c:7305)
  File "zmq/backend/cython/socket.pyx", line 683, in zmq.backend.cython.socket.Socket.send (zmq/backend/cython/socket.c:7048)
  File "zmq/backend/cython/socket.pyx", line 188, in zmq.backend.cython.socket._send_copy (zmq/backend/cython/socket.c:2815)
  File "zmq/utils/buffers.pxd", line 200, in zmq.utils.buffers.asbuffer_r (zmq/backend/cython/socket.c:8876)
  File "zmq/utils/buffers.pxd", line 159, in zmq.utils.buffers.asbuffer (zmq/backend/cython/socket.c:8295)
BufferError: memoryview: underlying buffer is not contiguous
ERROR:tornado.general:Uncaught exception, closing connection.

This could be dealt with a number of places:

1. by the user (not very user friendly)
2. by the widget developer (but this is a fairly generic problem)
3. in traittypes.Array.validate, by calling np.ascontiguousarray instead of np.asarray
4. or, as @jasongrout suggests:

Perhaps the underlying python/zmq interface should use something like PyMemoryView_GetContiguous to get a contiguous buffer.

@minrk any opinions on the matter?

[maartenbreddels]

Let me add to the discussion that if say ar is a contiguous array, you may want to be able to transfer ar.T, (just send along the strides and shape) but maybe not ar[::2], since it's a waste of memory/bandwidth, but both are flagged as C_CONTIGUOUS==False

[jasongrout]

@maartenbreddels - you'd probably want to encode the logic for ar.T vs ar at the serializer level (i.e., level 2 or 3), not at the ipykernel level. I think at the ipykernel level, we should either decide that we explicitly don't support non-contiguous buffers (status quo), or do a copy if necessary.

[maartenbreddels]

True, I think this can be solved at the serializer level, by sending the strides, and a new view of the data (the ar.T case). So nothing should change then?

[minrk]

ar.T is still contiguous memory (pyzmq requires PyBUF_ANY_CONTIGUOUS, not C_CONTIGUOUS), so that ought to send fine, as far as zmq is concerned (it's up to the higher-level code to send the appropriate strides, etc.).

(summarizing a bit from gitter):

• The most important thing here seems to be preventing an error like this from taking down the kernel
• introducing silent copy-if-not-contiguous at the zmq level is taking away too much control, especially for folks sending large data
• doing it at the jupyter_client level is probably also taking too much control away

We can definitely improve the error handling so that it's a user call and not an async one that hits the error. I opened zeromq/pyzmq#1011 to prevent the socket from being closed due to an unhandled error (not sure why were doing that, but it's ancient code, probably from early tornado copy/paste). I also opened jupyter/jupyter_client#258 to check contiguous memoryviews in Session.send, which should move this error into user-space. The last thing to consider is should there be a mechanism somewhere (in jupyter_client, ipykernel, widgets, or elsewhere) to give users/libraries the ability to more easily cast to contiguous memory where appropriate.