<div dir="ltr"><div dir="ltr">Hi Pavel, Gydo,<div><br></div><div>Here it is:</div><div><a href="https://github.com/cctbx/cctbx_project/commit/cc21afcf004aedfcfc71bc16d06738abf441c085">https://github.com/cctbx/cctbx_project/commit/cc21afcf004aedfcfc71bc16d06738abf441c085</a><br></div><div><br></div><div>Gydo, please note slightly different function name and correct you code accordingly.</div><div><br></div><div>Best regards,</div><div>Oleg Sobolev. </div></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 21, 2018 at 10:45 AM Pavel Afonine &lt;<a href="mailto:pafonine@lbl.gov">pafonine@lbl.gov</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF">
    Hi Oleg,<br>
    <br>
    I did not follow this conversation, so I don&#39;t know details and
    nuances. All I care about is making sure we don&#39;t have lines like<br>
    <br>
    if geo_box.get_source() == &#39;SCHRODINGER&#39;:<br>
    <br>
    in low-level codes (for reasons I hinted earlier). So if that gets
    removed one way or another I&#39;m happy!<br>
    <br>
    All the best,<br>
    Pavel<br>
    <br>
    <div class="gmail-m_-7842798474965407074moz-cite-prefix">On 12/21/18 10:36, Oleg Sobolev wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">
        <div dir="ltr">
          <div dir="ltr">Hi Pavel, Gydo,
            <div><br>
            </div>
            <div>I agree with Gydo&#39;s points. The best way forward would
              be to implement shift_cart() function for base geometry
              and actually treat reference coordinate restraints
              appropriately even in our basic GRM instead of throwing
              them away and getting biased estimation of weight.</div>
            <div><br>
            </div>
            <div>Secondly, we can remove the second call </div>
            <div>
              <div>      if geo_box.get_source() == &#39;SCHRODINGER&#39;:</div>
              <div>        geo_box.shift_cart(shift_back)</div>
            </div>
            <div>completely right now, because we throw away geo_box and
              don&#39;t need to care about shifting stuff back.</div>
            <div><br>
            </div>
            <div>Best regards,</div>
            <div>Oleg Sobolev.</div>
            <div><br>
            </div>
          </div>
        </div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr">On Fri, Dec 21, 2018 at 9:37 AM Gydo van Zundert
          &lt;<a href="mailto:gydo.vanzundert@schrodinger.com" target="_blank">gydo.vanzundert@schrodinger.com</a>&gt;
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div dir="ltr">
            <div>Thanks for the feedback Pavel. First off, let me start
              by saying I&#39;m feeling hesitant about sharing what I think
              is best, since I&#39;m obviously an outsider and you guys are
              the keepers of the code. Anyway ...<br>
            </div>
            <div><br>
            </div>
            <div>I agree that checking whether the geometry restraints
              manager is using Schrodinger restraints is not elegant and
              ideally should be removed. The issue here is that a part
              of the map is being boxed out corresponding to a chunk of
              the chain. However, this newly created box has its origin
              set to (0, 0, 0), which means that the chunk also needs to
              move accordingly if it is to stay in the same relative
              position of the density. This boxing behavior is leading
              to some quircks even in the original code, since it
              requires that the reference_coordinate restraints are
              removed from the geometry_restraints_manager during the
              weight determination of the chunk and the density (else
              &quot;disaster&quot; happens according to a comment :-) ).<br>
            </div>
            <div>I think its best to either set the origin of the box so
              that no coordinates or  restraints have to move or be
              removed. <br>
            </div>
            <div>It also got me wondering why the density is being boxed
              out, is there a (performance) reason for this being done?
              Since typically only the values under the atom coordinates
              or its surroundings are used, it should not really matter
              on the size of the map, since you can efficiently loop
              over the relevant voxels or do linear interpolation on the
              atom positions, something which you do anyway. You might
              even get a marginal performance increase, since some
              operations can be removed, by just using the original
              larger map.<br>
            </div>
            <div><br>
            </div>
            <div>Best,</div>
            <div>Gydo<br>
            </div>
          </div>
          <br>
          <div class="gmail_quote">
            <div dir="ltr">On Fri, Dec 21, 2018 at 11:59 AM Nigel
              Moriarty &lt;<a href="mailto:nwmoriarty@lbl.gov" target="_blank">nwmoriarty@lbl.gov</a>&gt;
              wrote:<br>
            </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              <div dir="ltr">Not sure if Gydo is seeing this but it may
                be outside his expertise. To me. it highlights a wrinkle
                in the code. The only reason for not shifting is so that
                the forcefields are consistent in the long range terms.
                I believe it could be avoided by moving the entire model
                such that the submodel is in the required position but
                the other atoms are in the correct relative position. 
                <div><br clear="all">
                  <div>
                    <div dir="ltr" class="gmail-m_-7842798474965407074gmail-m_6035888157603407385gmail-m_5003749320834457516gmail_signature">
                      <div dir="ltr">
                        <div>
                          <div dir="ltr">
                            <div dir="ltr">Cheers
                              <div><br>
                              </div>
                              <div>Nigel
                                <div><br>
                                </div>
                                <div>---</div>
                                <div>Nigel W. Moriarty<br>
                                  Building 33R0349, Molecular Biophysics
                                  and Integrated Bioimaging</div>
                                <div>Lawrence Berkeley National
                                  Laboratory<br>
                                  Berkeley, CA 94720-8235<br>
                                  Phone : 510-486-5709     Email :
                                  <a class="gmail-m_-7842798474965407074moz-txt-link-abbreviated" href="mailto:NWMoriarty@LBL.gov" target="_blank">NWMoriarty@LBL.gov</a><br>
                                  Fax   : 510-486-5909       Web  : <a href="http://CCI.LBL.gov" target="_blank">CCI.LBL.gov</a></div>
                              </div>
                            </div>
                          </div>
                        </div>
                      </div>
                    </div>
                  </div>
                  <br>
                </div>
              </div>
              <br>
              <div class="gmail_quote">
                <div dir="ltr">On Thu, Dec 20, 2018 at 5:02 PM Pavel
                  Afonine &lt;<a href="mailto:pafonine@lbl.gov" target="_blank">pafonine@lbl.gov</a>&gt;
                  wrote:<br>
                </div>
                <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                  <div bgcolor="#FFFFFF"> Bad idea to hard-wire package
                    name into lowest level code. Now imagine we need the
                    same for afit, amber, quantumbio, rosetta, etc.. Are
                    we going to have a page-long block of &quot;if&quot;
                    statements. Please re-think and remove. For example,
                    whether you want to shift it or not can be a
                    parameter that you cast way level up in the context
                    specific code.<br>
                    <br>
                    Pavel<br>
                    <br>
                    <div class="gmail-m_-7842798474965407074gmail-m_6035888157603407385gmail-m_5003749320834457516gmail-m_8321930562097640801moz-cite-prefix">On
                      12/20/18 09:53, CCTBX commit wrote:<br>
                    </div>
                    <blockquote type="cite"> <tt>Repository : <a class="gmail-m_-7842798474965407074gmail-m_6035888157603407385gmail-m_5003749320834457516gmail-m_8321930562097640801moz-txt-link-freetext">ssh://g18-sc-serv-04.diamond.ac.uk/cctbx</a><br>
                        On branch  : master<br>
                        <br>
                        <hr><br>
                        <br>
                        commit 1ad3fe05571162839e770c35ed032a7fc38a33b7<br>
                        Author: Gydo van Zundert <a class="gmail-m_-7842798474965407074gmail-m_6035888157603407385gmail-m_5003749320834457516gmail-m_8321930562097640801moz-txt-link-rfc2396E" href="mailto:gydo.vanzundert@schrodinger.com" target="_blank">&lt;gydo.vanzundert@schrodinger.com&gt;</a><br>
                        Date:   Wed Dec 19 14:58:36 2018 -0500<br>
                        <br>
                            Fix typo geobox<br>
                        <br>
                        <br>
                        <hr><br>
                        <br>
                        1ad3fe05571162839e770c35ed032a7fc38a33b7<br>
                        mmtbx/refinement/real_space/individual_sites.py
                        | 4 ++--<br>
                        1 file changed, 2 insertions(+), 2 deletions(-)<br>
                        <br>
                        diff --git
                        a/mmtbx/refinement/real_space/individual_sites.py
b/mmtbx/refinement/real_space/individual_sites.py<br>
                        index c61e88b01..20f38a6b0 100644<br>
                        <tt style="color:rgb(136,0,0)">---
                          a/mmtbx/refinement/real_space/individual_sites.py</tt><br>
                        <tt style="color:rgb(0,0,136)">+++
                          b/mmtbx/refinement/real_space/individual_sites.py</tt><br>
                        @@ -350,7 +350,7 @@ class
                        box_refinement_manager(object):<br>
                        <br>
                               # When using the Schrodinger force field,
                        move the whole structure as the<br>
                               # selected atoms are environment aware.<br>
                        <tt style="color:rgb(136,0,0)">-      if
                          geobox.get_source() == &#39;SCHRODINGER&#39;:</tt><br>
                        <tt style="color:rgb(0,0,136)">+      if
                          geo_box.get_source() == &#39;SCHRODINGER&#39;:</tt><br>
                                 geo_box.shift_cart(box.shift_cart)<br>
                        <br>
                               rsr_simple_refiner = simple(<br>
                        @@ -377,7 +377,7 @@ class
                        box_refinement_manager(object):<br>
                                 iselection, sites_cart_refined)<br>
       self.xray_structure.set_sites_cart(sites_cart_moving)<br>
                               self.sites_cart =
                        self.xray_structure.sites_cart()<br>
                        <tt style="color:rgb(136,0,0)">-      if
                          geobox.get_source() == &#39;SCHRODINGER&#39;:</tt><br>
                        <tt style="color:rgb(0,0,136)">+      if
                          geo_box.get_source() == &#39;SCHRODINGER&#39;:</tt><br>
                                 geo_box.shift_cart(shift_back)<br>
                             else: # NCS constraints are present<br>
                               # select on xrs, grm, ncs_groups<br>
                      </tt> <br>
                      <hr>
                      <p align="center">To unsubscribe from the
                        CCTBX-COMMIT list, click the following link:<br>
                        <a href="https://www.jiscmail.ac.uk/cgi-bin/webadmin?SUBED1=CCTBX-COMMIT&amp;A=1" target="_blank">https://www.jiscmail.ac.uk/cgi-bin/webadmin?SUBED1=CCTBX-COMMIT&amp;A=1</a>
                      </p>
                    </blockquote>
                    <br>
                  </div>
                  _______________________________________________<br>
                  cctbxbb mailing list<br>
                  <a href="mailto:cctbxbb@phenix-online.org" target="_blank">cctbxbb@phenix-online.org</a><br>
                  <a href="http://phenix-online.org/mailman/listinfo/cctbxbb" rel="noreferrer" target="_blank">http://phenix-online.org/mailman/listinfo/cctbxbb</a><br>
                </blockquote>
              </div>
              _______________________________________________<br>
              cctbxbb mailing list<br>
              <a href="mailto:cctbxbb@phenix-online.org" target="_blank">cctbxbb@phenix-online.org</a><br>
              <a href="http://phenix-online.org/mailman/listinfo/cctbxbb" rel="noreferrer" target="_blank">http://phenix-online.org/mailman/listinfo/cctbxbb</a><br>
            </blockquote>
          </div>
          _______________________________________________<br>
          cctbxbb mailing list<br>
          <a href="mailto:cctbxbb@phenix-online.org" target="_blank">cctbxbb@phenix-online.org</a><br>
          <a href="http://phenix-online.org/mailman/listinfo/cctbxbb" rel="noreferrer" target="_blank">http://phenix-online.org/mailman/listinfo/cctbxbb</a><br>
        </blockquote>
      </div>
      <br>
      <fieldset class="gmail-m_-7842798474965407074mimeAttachmentHeader"></fieldset>
      <br>
      <pre>_______________________________________________
cctbxbb mailing list
<a class="gmail-m_-7842798474965407074moz-txt-link-abbreviated" href="mailto:cctbxbb@phenix-online.org" target="_blank">cctbxbb@phenix-online.org</a>
<a class="gmail-m_-7842798474965407074moz-txt-link-freetext" href="http://phenix-online.org/mailman/listinfo/cctbxbb" target="_blank">http://phenix-online.org/mailman/listinfo/cctbxbb</a>
</pre>
    </blockquote>
    <br>
  </div>

</blockquote></div>