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

Oleg Sobolev osobolev at lbl.gov
Fri Dec 21 10:36:53 PST 2018


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 <
gydo.vanzundert at schrodinger.com> 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 <nwmoriarty at lbl.gov>
> 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 : NWMoriarty at LBL.gov
>> Fax   : 510-486-5909       Web  : CCI.LBL.gov
>>
>>
>> On Thu, Dec 20, 2018 at 5:02 PM Pavel Afonine <pafonine at lbl.gov> 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 <gydo.vanzundert at schrodinger.com>
>>> <gydo.vanzundert at schrodinger.com>
>>> 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
>>> cctbxbb at phenix-online.org
>>> http://phenix-online.org/mailman/listinfo/cctbxbb
>>>
>> _______________________________________________
>> cctbxbb mailing list
>> cctbxbb at phenix-online.org
>> http://phenix-online.org/mailman/listinfo/cctbxbb
>>
> _______________________________________________
> 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/20181221/14ae6309/attachment.htm>


More information about the cctbxbb mailing list