Opened 7 years ago

Closed 7 years ago

#1378 closed defect (fixed)

Bounds calculation fails under certain conditions

Reported by: Tristan Croll Owned by: Tom Goddard
Priority: critical Milestone:
Component: Graphics Version:
Keywords: Cc:
Blocked By: Blocking:
Notify when closed: Platform: all
Project: ChimeraX

Description

Root cause first: Places.array() returns a float64 array, while _geometry.point_copies_bounds requires float32. Changing bounds.point_bounds()` as per below gets me working again.

Symptoms: only crops up for me under the following circumstances: (a) the effective position of a model is not the identity; (b) there is a drawing visible that is *not* using shift_and_scale_array for its positions; and (c) coordinates are continually updating (i.e. I have a simulation running). Under those circumstances the drawing bounds calculation returns extreme values, so ZoomMouseMode sends the model flying into +/- infinity.

def point_bounds(xyz, placements=None):
    '''
    Return :py:class:`.Bounds` for a set of points, optionally
    multiple positions given by a :py:class:`.Places` instance.
    '''
    if len(xyz) == 0:
        return None

    if placements:
        from ._geometry import point_copies_bounds
        from numpy import require, float32
        xyz_min, xyz_max = point_copies_bounds(xyz, require(placements.array(), dtype=float32))
    else:
        from ._geometry import point_bounds
        xyz_min, xyz_max = point_bounds(xyz)

    b = Bounds(xyz_min, xyz_max)
    return b

Change History (5)

in reply to:  1 ; comment:1 by tic20@…, 7 years ago

Wait, no. That *doesn't* fix it.

On 2018-10-23 15:40, ChimeraX wrote:

comment:2 by Tristan Croll, 7 years ago

Sorry about the false start. Thought I'd tested my change, but turns out I'd forgotten to actually apply a non-identity transform to the top level model. Here's what I know:

  • when there's a non-identity transform in the model hierarchy, *and* a drawing where d.positions(displayed_only) returns a single transform, *and* I have a simulation running (that is, actively updating coordinates)...
m = session.isolde.selected_model
from chimerax.core.geometry import rotation
m.position=rotation((0,1,0), 90, center=m.atoms.coords.mean(axis=0))
from chimerax.isolde import session_extensions as sx
prm = sx.get_position_restraint_mgr(m)
pd = prm._pin_drawing
pd.get_positions(displayed_only=True).array()
Out[76]: 
array([[[  1.        ,  -0.53777278,   0.79855295,  81.43191528],
        [  0.88197355,   1.        ,   0.42942637, 108.42781067],
        [ -0.38601688,   0.82041745,   1.        ,  63.30295944]]])

pd.get_scene_positions(displayed_only=True).array()
Out[77]: 
array([[[ 1.43745782e+13,  1.11955185e+14,  1.00000358e+00,
          8.04532434e+01],
        [ 1.14367682e+14,  1.00000000e+00,  6.10875872e+11,
          1.08427811e+02],
        [-9.98404101e-01, -1.14367682e+14, -3.22517444e+10,
          5.68178342e+01]]])

# ... but the moment I pause the simulation, I get something that appears more reasonable but is still incorrect:
pd.get_scene_positions(displayed_only=True).array()
Out[78]: 
array([[[ -0.38601688,   0.82041745,   1.        ,  80.45324337],
        [  0.88197355,   1.        ,   0.42942637, 108.42781067],
        [ -1.        ,   0.53777278,  -0.79855295,  56.81783419]]])

# If I add one or more extra positions, the result appears correct whether or not a simulation is running:

pd.get_scene_positions(displayed_only=True).array()
Out[79]: 
array([[[ 1.11022302e-016,  4.59531485e-316,  1.00000000e+000,
          7.84171258e+001],
        [ 4.58711257e-316,  1.00000000e+000,  4.59215362e-316,
          1.08532318e+002],
        [-1.00000000e+000, -4.58102884e-316,  1.11022302e-016,
          6.05663388e+001]],

       [[ 1.11022302e-016,  4.61345536e-316,  1.00000000e+000,
          8.04532434e+001],
        [ 4.60751787e-316,  1.00000000e+000,  4.60936291e-316,
          1.08427811e+002],
        [-1.00000000e+000, -4.60663014e-316,  1.11022302e-016,
          5.68178342e+001]]])

comment:3 by Tristan Croll, 7 years ago

Resolution: nonchimerax
Status: assignedclosed

... ah. Poking at this some more, it looks like I might have a memory error elsewhere in my code that just happens to be impacting here. If I do:

pd.get_scene_positions(displayed_only=False)

... then I get the correct answer, but immediately following up with displayed_only=True (which should just re-use the stored value) then I get rubbish again. The problem appears to go away if I turn off my atomic symmetry display, which should be entirely unrelated. Suggests it's almost certainly my fault.

comment:4 by Tristan Croll, 7 years ago

Resolution: nonchimerax
Status: closedreopened

Belay that. After confusing the hell out of myself, the culprit turns out to be in Places.array() after all:

    def array(self):
        pa = self._place_array
        if pa is None:
            from numpy import empty, float64
            pa = empty((len(self),3,4), float64)
            if self._place_list is not None:
                for i,p in enumerate(self._place_list):
                    pa[i,:,:] = p.matrix
            elif self._shift_and_scale is not None:
>               pa[:]=0
                sas = self._shift_and_scale
                pa[:,:,3] = sas[:,:3]
                pa[:,0,0] = pa[:,1,1] = pa[:,2,2] = sas[:,3]
            elif self._opengl_array is not None:
                pa[:] = self._opengl_array.transpose((0,2,1))[:,:3,:]
            self._place_array = pa
        return pa

... because otherwise all off-diagonal elements of the rotation matrices are uninitialised.

comment:5 by Tom Goddard, 7 years ago

Resolution: fixed
Status: reopenedclosed

Fixed.

I'm amazed this mistake didn't effect more code, but I guess shift and scale instancing is mostly optimized so that 3x4 arrays are never produced.

Note: See TracTickets for help on using tickets.