[cctbxbb] [git/cctbx] master: Fix typo geobox (1ad3fe055)
Pavel Afonine
pafonine at lbl.gov
Sat Dec 22 14:58:07 PST 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/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
> <mailto: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
>> <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
>> <mailto: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 <mailto: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/20181222/a9553e23/attachment-0001.htm>
More information about the cctbxbb
mailing list