[cctbxbb] rt_mx inconsistencies

Nathaniel Echols 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)?

http://cctbx.svn.sourceforge.net/viewvc/cctbx/trunk/cctbx/sgtbx/space_group.cpp?r1=15082&r2=15658&sortby=rev

-Nat

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
> non-rotations.
> Let's r1 and r2 be rotations and m1 and m2 be non-rotations. After that
> revision,
>         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
> algorithm.
>
> What do you think?
>
> Best wishes,
>
> Luc Bourhis
>
> _______________________________________________
> cctbxbb mailing list
> cctbxbb at phenix-online.org
> http://phenix-online.org/mailman/listinfo/cctbxbb
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://phenix-online.org/pipermail/cctbxbb/attachments/20120718/8810e394/attachment.htm>


More information about the cctbxbb mailing list