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)
comment:2 by , 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 , 7 years ago
| Resolution: | → nonchimerax |
|---|---|
| Status: | assigned → closed |
... 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 , 7 years ago
| Resolution: | nonchimerax |
|---|---|
| Status: | closed → reopened |
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 , 7 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
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.