[cctbxbb] [git/cctbx] master: Fix typo geobox (1ad3fe055)
Pavel Afonine
pafonine at lbl.gov
Fri Dec 21 10:45:19 PST 2018
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
> <gydo.vanzundert at schrodinger.com
> <mailto: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 <mailto: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 <http://CCI.LBL.gov>
>
>
> On Thu, Dec 20, 2018 at 5:02 PM Pavel Afonine
> <pafonine at lbl.gov <mailto: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>
>> <mailto: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 <mailto:cctbxbb at phenix-online.org>
> http://phenix-online.org/mailman/listinfo/cctbxbb
>
> _______________________________________________
> cctbxbb mailing list
> cctbxbb at phenix-online.org <mailto:cctbxbb at phenix-online.org>
> http://phenix-online.org/mailman/listinfo/cctbxbb
>
> _______________________________________________
> cctbxbb mailing list
> cctbxbb at phenix-online.org <mailto: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/910ea41c/attachment-0001.htm>
More information about the cctbxbb
mailing list