"TypeError: Require contiguous 3x4 array" when Drawing has only one instance

[Tristan Croll]

Seems to be another change in the Drawing API that I'm struggling to come to terms with. In ISOLDE I'm now getting errors that crash the graphics loop any time I do something that creates exactly one instance of one of my restraint indicators (traceback below). Tracing it back using my dihedral restraints indicator as an example, the core of the problem seems to arise in the following snippet (from line 736 of drawing.py):

        pos = self.positions
        use_instancing = (len(pos) > 1 or pos.shift_and_scale_array() is not None)
        spos = self.parent.get_scene_positions(displayed_only=True) if use_instancing else self.get_scene_positions(displayed_only=True)
        for p in spos:
            # TODO: Optimize this to use same 4x4 opengl matrix each call.
            renderer.set_model_matrix(p)

If I have a look at the offending drawing:

from chimerax.isolde import session_extensions as sx

prm = sx.get_proper_dihedral_restraint_mgr(session.isolde.selected_model)

d = prm._ring_drawing

dpos = d.get_scene_positions(displayed_only=True)

dpos[0].matrix.data.c_contiguous
Out[5]: False

dpos[0].matrix.data.f_contiguous
Out[6]: True

ppos = d.parent.get_scene_positions(displayed_only=True)

ppos[0].matrix.data.c_contiguous
Out[8]: True

... so the drawing and parent have different row ordering. In the cases leading to the problem, use_instancing should be True (since len(pos) is 1 and pos.shift_and_scale_array() returns None).

That's about as far as I've gotten. The parent is a Model subclass which never modifies its own position - just handles the redrawing triggers, selection etc.. Just to be explicit about it, everything works exactly as expected when the drawing has 0 or >1 positions.

Traceback (most recent call last):
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/updateloop.py", line 72, in draw_new_frame
view.draw(check_for_changes = False)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/graphics/view.py", line 159, in draw
self._draw_scene(camera, drawings)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/graphics/view.py", line 217, in _draw_scene
draw_opaque(r, opaque_drawings)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/graphics/drawing.py", line 1355, in draw_opaque
_draw_multiple(drawings, renderer, Drawing.OPAQUE_DRAW_PASS)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/graphics/drawing.py", line 1366, in _draw_multiple
d.draw(renderer, draw_pass)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/graphics/drawing.py", line 683, in draw
self.draw_self(renderer, draw_pass)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/graphics/drawing.py", line 690, in draw_self
self._draw_geometry(renderer, opaque_only = any_transp)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/graphics/drawing.py", line 741, in _draw_geometry
renderer.set_model_matrix(p)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/graphics/opengl.py", line 608, in set_model_matrix
if ((model_matrix.is_identity() and cmm.is_identity())
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/geometry/place.py", line 298, in is_identity
or _geometry.is_identity_matrix(self.matrix, tolerance))
TypeError: Require contiguous 3x4 array

[Tristan Croll]

... sorry - use_instancing should of course be False.

[Tristan Croll]

If it helps, the transforms for both this drawing and a second drawing representing the "posts" are generated by the following:

    def _annotation_transforms(self):
        n = len(self)
        f = c_function('proper_dihedral_restraint_annotation_transform',
            args = (ctypes.c_void_p, ctypes.c_size_t, ctypes.c_void_p, ctypes.c_void_p))
        tf1 = numpy.empty((n,4,4), float64)
        tf2 = numpy.empty((n,4,4), float64)
        f(self._c_pointers, n, pointer(tf1), pointer(tf2))
        from chimerax.core.geometry import Places
        return (Places(opengl_array=tf1), Places(opengl_array=tf2))

[goddard@…]

I have been optimizing 3x4 matrix code, doing some calculations in C++ last week.  The C++ wants a contiguous array and your example does not give that.  I will fix it today.

[Tom Goddard]

The problem is likely because you Places(opengl_array=tf) requires a float32 numpy array and you gave it a float64 array. I've added type checking in the Places constructor to throw an error if the wrong type array is given as an argument.

I'm not entirely sure that is your problem since I cannot reproduce your error. It may be that you are using an older ChimeraX that did not convert non-contiguous to contiguous arrays.

Please use the Report a Bug tool when a traceback occurs so we know exactly the ChimeraX version and platform.

[Tristan Croll]

I've changed my code to use float32 arrays for transforms, but that wasn't the problem. The root cause is in Places.place_list() (called by Places.__getitem__) when the Places object was created from an openGL array:

            elif self._opengl_array is not None:
                pl = tuple(Place(m.transpose()[:3, :])
                           for m in self._opengl_array)

The following corrects it:

            elif self._opengl_array is not None:
                from numpy import ascontiguousarray
                pl = tuple(Place(ascontiguousarray(m.transpose()[:3, :]))
                           for m in self._opengl_array)

[Tristan Croll]

Adding the argument order='C' in Place.__init__() at line 74 also does it nice and simply.

    def __init__(self, matrix=None, axes=None, origin=None):
        '''Coordinate axes and origin can be specified as a 3 by 4 array
        where the first 3 columns are the axes vectors and the fourth
        column is the origin.  Alternatively axes can be specified as
        a list of axes vectors and the origin can be specifed as a vector.
        '''
        from numpy import array, float64, transpose
        if matrix is None:
            m = array(((1, 0, 0, 0), (0, 1, 0, 0), (0, 0, 1, 0)), float64)
            if axes is not None:
                m[:, :3] = transpose(axes)
            if origin is not None:
                m[:, 3] = origin
        else:
            m = array(matrix, float64, order='C')

[Tom Goddard]

Fixed. Put in order = 'C' in Place constructor.

Interesting. I looked at exactly this code yesterday for this bug report and concluded that the array(matrix, float64) call in the Place constructor makes a copy of the array so it must be contiguous and I assumed numpy always allocates a new array as C-order contiguous. But I just tested and the numpy array() docs makes this clear that if array is called on an F-contiguous array then array() produces an F-contiguous copy. And in your case because the C-contiguous array was transposed it is F-contiguous.