The math is lost on me, but is it possible that revision 15658 solves the problem (in part or in whole)?<div><br></div><div><a href="http://cctbx.svn.sourceforge.net/viewvc/cctbx/trunk/cctbx/sgtbx/space_group.cpp?r1=15082&r2=15658&sortby=rev">http://cctbx.svn.sourceforge.net/viewvc/cctbx/trunk/cctbx/sgtbx/space_group.cpp?r1=15082&r2=15658&sortby=rev</a><br>
<br></div><div>-Nat</div><div><br><div class="gmail_quote">On Wed, Jul 18, 2012 at 9:31 AM, Luc Bourhis <span dir="ltr"><<a href="mailto:luc_j_bourhis@mac.com" target="_blank">luc_j_bourhis@mac.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi fellow cctbx developers,<br>
<br>
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.<br>
<br>
(1) However, revision 15082 has introduced an inconsistency in the comparison of rt_mx instances related to that divide between rotations and non-rotations.<br>
Let's r1 and r2 be rotations and m1 and m2 be non-rotations. After that revision,<br>
� � � � r1 == r2 works all right<br>
� � � � r1 == m1 throws an exception<br>
� � � � m1 == m2 throws an exception<br>
As far as I know the problem has been first reported by Phil Evans who provided a very clear regression test.<br>
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).<br>
<br>
(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.<br>
<br>
(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.<br>
<br>
So I am coming to you to discuss what to do.<br>
<br>
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:<br>
(i) Remove operator <=, >=, <, > from class rt_mx and revert operator== to what it used to be.<br>
(ii) Change rot_mx and tr_vec operator== to disregard the value of den()<br>
(iii) Introduce a proxy class that wraps around rt_mx and that obeys the ordering introduced by revision 14990<br>
(iv) Rewrite the implementation of �the symmetry-equivalent pair interaction table generation to use that proxy instead of directly comparing space group symmetry elements.<br>
<br>
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.<br>
<br>
What do you think?<br>
<br>
Best wishes,<br>
<br>
Luc Bourhis<br>
<br>
_______________________________________________<br>
cctbxbb mailing list<br>
<a href="mailto:cctbxbb@phenix-online.org">cctbxbb@phenix-online.org</a><br>
<a href="http://phenix-online.org/mailman/listinfo/cctbxbb" target="_blank">http://phenix-online.org/mailman/listinfo/cctbxbb</a><br>
</blockquote></div><br></div>