Re: [cctbxbb] [git/cctbx] master: Fix typo geobox (1ad3fe055)

22 Dec 2018


      Looks great! Thanks Oleg.
Pavel

On 12/21/18 12:39, Oleg Sobolev wrote:
...
Hi Pavel, Gydo,
Here it is:
https://github.com/cctbx/cctbx_project/commit/cc21afcf004aedfcfc71bc16d06738...
Gydo, please note slightly different function name and correct you 
code accordingly.
Best regards,
Oleg Sobolev.
On Fri, Dec 21, 2018 at 10:45 AM Pavel Afonine mailto:[email protected]> wrote:
Hi Oleg,
I did not follow this conversation, so I don't know details and
    nuances. All I care about is making sure we don't have lines like
if geo_box.get_source() == 'SCHRODINGER':
in low-level codes (for reasons I hinted earlier). So if that gets
    removed one way or another I'm happy!
All the best,
    Pavel
On 12/21/18 10:36, Oleg Sobolev wrote:
...
Hi Pavel, Gydo,
I agree with Gydo'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.
Secondly, we can remove the second call
          if geo_box.get_source() == 'SCHRODINGER':
            geo_box.shift_cart(shift_back)
    completely right now, because we throw away geo_box and don't
    need to care about shifting stuff back.
Best regards,
    Oleg Sobolev.
On Fri, Dec 21, 2018 at 9:37 AM Gydo van Zundert
    mailto:[email protected]> wrote:
Thanks for the feedback Pavel. First off, let me start by
        saying I'm feeling hesitant about sharing what I think is
        best, since I'm obviously an outsider and you guys are the
        keepers of the code. Anyway ...
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 "disaster" happens
        according to a comment :-) ).
        I think its best to either set the origin of the box so that
        no coordinates or  restraints have to move or be removed.
        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.
Best,
        Gydo
On Fri, Dec 21, 2018 at 11:59 AM Nigel Moriarty
        mailto:[email protected]> wrote:
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.
Cheers
Nigel
---
            Nigel W. Moriarty
            Building 33R0349, Molecular Biophysics and Integrated
            Bioimaging
            Lawrence Berkeley National Laboratory
            Berkeley, CA 94720-8235
            Phone : 510-486-5709     Email : [email protected]
            mailto:[email protected]
            Fax   : 510-486-5909 Web  : CCI.LBL.gov http://CCI.LBL.gov
On Thu, Dec 20, 2018 at 5:02 PM Pavel Afonine
            mailto:[email protected]> wrote:
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 "if" 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.
Pavel
On 12/20/18 09:53, CCTBX commit wrote:
...
Repository : ssh://g18-sc-serv-04.diamond.ac.uk/cctbx
                On branch  : master
------------------------------------------------------------------------
commit 1ad3fe05571162839e770c35ed032a7fc38a33b7
                Author: Gydo van Zundert
                
                mailto:[email protected]
                Date:   Wed Dec 19 14:58:36 2018 -0500
    Fix typo geobox
------------------------------------------------------------------------
1ad3fe05571162839e770c35ed032a7fc38a33b7
                mmtbx/refinement/real_space/individual_sites.py | 4 ++--
                1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
                a/mmtbx/refinement/real_space/individual_sites.py
                b/mmtbx/refinement/real_space/individual_sites.py
                index c61e88b01..20f38a6b0 100644
                --- a/mmtbx/refinement/real_space/individual_sites.py
                +++ b/mmtbx/refinement/real_space/individual_sites.py
                @@ -350,7 +350,7 @@ class
                box_refinement_manager(object):
       # When using the Schrodinger force field,
                move the whole structure as the
                       # selected atoms are environment aware.
                -      if geobox.get_source() == 'SCHRODINGER':
                +      if geo_box.get_source() == 'SCHRODINGER':
                         geo_box.shift_cart(box.shift_cart)
       rsr_simple_refiner = simple(
                @@ -377,7 +377,7 @@ class
                box_refinement_manager(object):
                         iselection, sites_cart_refined)
                       self.xray_structure.set_sites_cart(sites_cart_moving)
                       self.sites_cart =
                self.xray_structure.sites_cart()
                -      if geobox.get_source() == 'SCHRODINGER':
                +      if geo_box.get_source() == 'SCHRODINGER':
                         geo_box.shift_cart(shift_back)
                     else: # NCS constraints are present
                       # select on xrs, grm, ncs_groups
------------------------------------------------------------------------
To unsubscribe from the CCTBX-COMMIT list, click the
                following link:
                https://www.jiscmail.ac.uk/cgi-bin/webadmin?SUBED1=CCTBX-COMMIT&A=1
_______________________________________________
                cctbxbb mailing list
                [email protected]
                mailto:[email protected]
                http://phenix-online.org/mailman/listinfo/cctbxbb
_______________________________________________
            cctbxbb mailing list
            [email protected] mailto:[email protected]
            http://phenix-online.org/mailman/listinfo/cctbxbb
_______________________________________________
        cctbxbb mailing list
        [email protected] mailto:[email protected]
        http://phenix-online.org/mailman/listinfo/cctbxbb
_______________________________________________
    cctbxbb mailing list
    [email protected] mailto:[email protected]
    http://phenix-online.org/mailman/listinfo/cctbxbb