[cctbxbb] [git/cctbx] master: Fix typo geobox (1ad3fe055)
Oleg Sobolev
osobolev at lbl.gov
Fri Dec 21 12:39:27 PST 2018
Hi Pavel, Gydo,
Here it is:
https://github.com/cctbx/cctbx_project/commit/cc21afcf004aedfcfc71bc16d06738abf441c085
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 <pafonine at lbl.gov> 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 <
> 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
>>
>
>
> _______________________________________________
> cctbxbb mailing listcctbxbb at phenix-online.orghttp://phenix-online.org/mailman/listinfo/cctbxbb
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://phenix-online.org/pipermail/cctbxbb/attachments/20181221/179979f4/attachment.htm>
More information about the cctbxbb
mailing list