BUG: Set default pbc sel flag to False

[tylerjereddy]

Fixes #1795 and adds corresponding unit test. See the associated issue for more detailed analysis & discussion.

In short, we now set (and test that we set) use_periodic_selections flag to False by default (it was True by default).

Longer term, flags are to be removed and the unit test (etc.) will need to be mutated accordingly. There's also (confusingly) a use_pbc flag that is set (and tested to be set) to False by default, but that is not used for the affected code in the original issue. That should perhaps be cleaned up, but we're planning to remove flags eventually anyway so perhaps better to hold over to that effort.

[richardjgowers]

I'm not sure this is the right fix. By changing a default won't we cause differences with all selections going forwards?

[richardjgowers]

@tylerjereddy I think all that is needed is to remove the pbc=periodic addition to center_of_gravity

[tylerjereddy]

@richardjgowers Can you provide a simple test case / example where detection of the issue & correct outcome is obvious? If we added support for pbc sphzone stuff at some point in the past, there should have been a unit test to verify that it would make the correct selection if there's a straddling issue or whatever.

[tylerjereddy]

Even a small / crude toy example could be useful if we use i.e., an empty universe to start with. It might take me a while before I get around to making one & the pbc stuff can be mind-bending to think about.

I should also point out that the docs seem a bit misleading because we imply that use_periodic_selections defaults to False in the selections docs but say that it defaults to True in the flags docs.

[tylerjereddy]

One possible test might be something like below, where X is the "protein" and W are the "waters." Distinguishing between W3 & W4 in a unit test selection would partially clarify that we have a spherical selection wrapped about the boundary.

However, if the coordinates of X are split over the periodic boundary, I'm afraid our documentation for sphzone clearly indicates that the centroid of X's coordinates is to be used for generating the sphere:

sphzone externalRadius selection
selects all atoms within a spherical zone centered in the center of geometry (COG) of a given selection, e.g. sphzone 6.0 ( protein and ( resid 130 or resid 80 ) ) selects the center of geometry of protein, resid 130, resid 80 and creates a sphere of radius 6.0 around the COG.

Those docs will need an overhaul or expansion to include PBC as well if we go that route.

[tylerjereddy]

One might encourage a user to use wrapping (see: #246 ) before sphzone, but that's something that's currently up to them, I think.

[richardjgowers]

Yeah so our "pbc" flag doesn't correctly do wrapping, so currently a group of [W2, W3] will fail miserably in most selections.

As far as fixing regressions in #1795 (ie this PR) we just need to take out the pbc kwarg in the selection. This then assumes that the user is working with a properly wrapped Universe.

More generally we've got #1185 and #1760 which are about how pbc=True/False isn't enough and we probably need some wrapping functions which will be used in selections too.

[richardjgowers]

#1795 is now fixed, flags are also deprecated now