Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#1233 closed defect (not a bug)

Grid_Data applies rotation and skew in wrong order?

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

Description

Unless I'm missing something obvious...

The background is that while ChimeraX stores its volume data as (k,j,i), Clipper stores as (i,j,k). Previously I had a somewhat ugly approach on the C++ side to handle the copying, but to make things cleaner with the new bindings I decided to instead instantiate the Array_Grid_Data with rotation=((0,0,1),(0,1,0),(1,0,0)). That works fine for my default test case, but other maps were coming out with strange offsets to the model. Eventually pinned it down to cell angles - my default test is a perfect (90,90,90), and the failing maps are all non-orthogonal on at least one angle ((90, 100.12, 90) for the case I first noticed the problem on).

The upshot is that I think in Grid_Data.update_transform() the rotation and skew are being multiplied in the wrong order (r*s rather than s*r). Swapping the order as per the below puts everything back in registration:

  def update_transform(self):

    from chimerax.core.geometry import place
    saxes = place.skew_axes(self.cell_angles)
    from numpy import array
    rsaxes = saxes*array(self.rotation)
    tf, tf_inv = transformation_and_inverse(self.origin, self.step, rsaxes)
    if (self.ijk_to_xyz_transform is None or not tf.same(self.ijk_to_xyz_transform) or
        self.xyz_to_ijk_transform is None or not tf_inv.same(self.xyz_to_ijk_transform)):
      self.ijk_to_xyz_transform = tf
      self.xyz_to_ijk_transform = tf_inv
      self.coordinates_changed()


Change History (2)

comment:1 by Tom Goddard, 7 years ago

Resolution: not a bug
Status: assignedclosed

The rotation in Grid_Data is not for permuting axes, it is for rotating the entire map, therefore it is done after skewing not before skewing and the current code is correct for the intended purpose.

It is simple to reverse the axes in numpy

mperm = numpy.transpose(m)

and this operation does not copy the data just gives a new array with different strides.

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

Ah - I *had* actually tried that, but now I realise that I neglected to 
retain a view on the original grid for refilling purposes. Working as 
expected now.

On 2018-08-03 17:57, ChimeraX wrote:
Note: See TracTickets for help on using tickets.