[cctbxbb] rt_mx inconsistencies
nechols at lbl.gov
Wed Jul 18 06:39:15 PDT 2012
The math is lost on me, but is it possible that revision 15658 solves the
problem (in part or in whole)?
On Wed, Jul 18, 2012 at 9:31 AM, Luc Bourhis <luc_j_bourhis at mac.com> wrote:
> Hi fellow cctbx developers,
> class rt_mx, contrarily to the documentation, does not only hold rotation
> + translation but any transformation y = M x + t where M is an invertible
> matrix. As a result, rt_mx can be used to store both space group elements
> and change-of-basis operators (the latter are in general not rotations).
> Not the purest design but apart from the recurrent need of improving on the
> documentation, it has been all right.
> (1) However, revision 15082 has introduced an inconsistency in the
> comparison of rt_mx instances related to that divide between rotations and
> Let's r1 and r2 be rotations and m1 and m2 be non-rotations. After that
> r1 == r2 works all right
> r1 == m1 throws an exception
> m1 == m2 throws an exception
> As far as I know the problem has been first reported by Phil Evans who
> provided a very clear regression test.
> The reason is simple: that revision changed operator== to compare the
> geometric elements, assuming the compared operators are rotations (and
> therefore that they have an invariant axis or plane).
> (2) Another side effect of revision 15082 seems to result in r1 == r2
> being true even if r1.t() and r2.t() do not have the same denominator.
> Before that revision, this comparison used to return false. As part of that
> revision, one test has been changed from != to == for that very reason.
> (3) revision 15082 follows revision 14990 that introduces an order on
> rt_mx intances. That order is used to implement a new symmetry-equivalent
> pair interaction table and rightly so op1 == op2 is made consistent with
> op1 < op2 and op1 <= op2 by revision 15082.
> So I am coming to you to discuss what to do.
> I am of the opinion that (1) needs fixing. I am also of the opinion that
> (2) is a good move. Here is what I propose to do:
> (i) Remove operator <=, >=, <, > from class rt_mx and revert operator== to
> what it used to be.
> (ii) Change rot_mx and tr_vec operator== to disregard the value of den()
> (iii) Introduce a proxy class that wraps around rt_mx and that obeys the
> ordering introduced by revision 14990
> (iv) Rewrite the implementation of the symmetry-equivalent pair
> interaction table generation to use that proxy instead of directly
> comparing space group symmetry elements.
> Basically I propose a very orthogonal design that keeps the core sgtbx
> pristine except for the fixing of a small but long-standing glitch while
> encapsulating the feature needed by one single algorithm along with that
> What do you think?
> Best wishes,
> Luc Bourhis
> cctbxbb mailing list
> cctbxbb at phenix-online.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cctbxbb