[cctbxbb] model selection

[Pavel Afonine]

Agreed about not constructing what's already available and costly to 
construct!

Speaking of consistency, 'selection' of 'atom_selection_cache' uses 
'string' as the argument, not selstr as in 'model', example:

mmtbx/utils/tst_switch_rotamers.py:   sel = 
ph.atom_selection_cache().selection(string = "not resname TYR")

I see the different between these flavors of select*. Ok, I afraid I'll 
have to read the code each time I use these..

Pavel

On 8/16/18 16:01, Oleg Sobolev wrote:
> Hi Pavel,
>
>     Currently model class has three selection methods:
>
>     def selection(self, selstr, optional=True):
>
> Takes selection string, returns bool selection
>
>     def select(self, selection):
>
> Takes selection (bool or iselection), applies it to self and returns 
> selected part of model object.
>
>     def iselection(self, selstr):
>
> Same as first, but returns iselection.
>
>
>     which is hideously confusing with zero chance to remember, and
>     also inconsistent with the rest.
>
> Let me disagree on the point of inconsistency with the rest.
> selection and iselection functions are available with exactly same 
> meaning for atom_selection_cache.
> select is available with exact same meaning (returning part of an 
> object) in hierarchy, af_shared_atom, flex.vec3_double, xray_structure 
> etc which you pointed out.
>
>     Can we consolidate all three into one ".select()" that would allow
>     taking bool or int or str?
>
> No, because they are doing different things. Selection and iselection 
> wraps atom_selection_cache functionality and not duplicating those of 
> select() function. ".select()" function already has the described 
> functionality.
>
>     Or you think there are philosophical arguments against it?
>
> Yes, and practical ones as well. I'm caching atom_selection_cache 
> inside model object, so you don't have to construct it again if you 
> use wrappers available in model class. 
> hierarchy.atom_selection_cache() is not a free function to the point I 
> had to pass the cache around to avoid constructing it again to achieve 
> measurable speedup. Especially for larger models.
>
> At the very least, please use model.get_atom_selection_cache() instead 
> of constructing it.
>
> Best regards,
> Oleg Sobolev.
>
>
>     Pavel
>
>     On 8/16/18 15:00, CCTBX commit wrote:
>>     Repository : ssh://g18-sc-serv-04.diamond.ac.uk/cctbx
>>     On branch  : master
>>
>>     ------------------------------------------------------------------------
>>
>>
>>     commit c98b44997e8eb7bf53ee828f510b933cd5167758
>>     Author: Oleg Sobolev <osobolev at lbl.gov> <mailto:osobolev at lbl.gov>
>>     Date:   Thu Aug 16 15:00:20 2018 -0700
>>
>>         Proper use of model class
>>
>>
>>     ------------------------------------------------------------------------
>>
>>
>>     c98b44997e8eb7bf53ee828f510b933cd5167758
>>     mmtbx/model/model.py | 3 +--
>>     1 file changed, 1 insertion(+), 2 deletions(-)
>>
>>     diff --git a/mmtbx/model/model.py b/mmtbx/model/model.py
>>     index 6f2247452..87412f95c 100644
>>     --- a/mmtbx/model/model.py
>>     +++ b/mmtbx/model/model.py
>>     @@ -2312,8 +2312,7 @@ class manager(object):
>>          sizes = flex.int <http://flex.int>()
>>          h = self.get_hierarchy()
>>          if(macro_molecule_only):
>>     -      asc = h.atom_selection_cache()
>>     -      s = asc.selection("protein or nucleotide")
>>     +      s = self.selection("protein or nucleotide")
>>            h = h.select(s)
>>          for r in h.residue_groups():
>>            sizes.append(r.atoms().size())
>
>
>     _______________________________________________
>     cctbxbb mailing list
>     cctbxbb at phenix-online.org <mailto:cctbxbb at phenix-online.org>
>     http://phenix-online.org/mailman/listinfo/cctbxbb
>     <http://phenix-online.org/mailman/listinfo/cctbxbb>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://phenix-online.org/pipermail/cctbxbb/attachments/20180816/de1008ca/attachment-0001.htm>