Skip to content

Add cylindrical selection test using PeriodicKDTree #5194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rishinamdev17 wants to merge 6 commits into
base: develop
Choose a base branch
from
+31 −0
Open

Add cylindrical selection test using PeriodicKDTree #5194

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff13d12
Add cylindrical selection test using PeriodicKDTree
rishinamdev17 Dec 30, 2025
f58bd52
Format cylindrical selection test with pinned Black version
rishinamdev17 Jan 1, 2026
d05ed3d
Add sanity test for cylayer selection using COG
rishinamdev17 Jan 2, 2026
f61e50e
Format cylayer selection test for Black
rishinamdev17 Jan 2, 2026
b7c3bb6
Address review feedback for cylayer selection test
rishinamdev17 Jan 4, 2026
12d7d6e
Test cylayer selection against PeriodicKDTree reference
rishinamdev17 Jan 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 38 additions & 59 deletions testsuite/MDAnalysisTests/core/test_atomselections.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from MDAnalysis import SelectionError, SelectionWarning
from MDAnalysis.core.selection import Parser
from MDAnalysis.lib.distances import distance_array
from MDAnalysis.lib.pkdtree import PeriodicKDTree
from MDAnalysis.tests.datafiles import (
DCD,
GRO,
Expand All @@ -56,7 +57,7 @@
waterPSF,
)
from numpy.lib import NumpyVersion
from numpy.testing import assert_equal
from numpy.testing import assert_array_equal, assert_equal

from MDAnalysisTests import make_Universe

Expand Down Expand Up @@ -127,9 +128,7 @@ def test_resid_single(self, universe):
def test_resid_range(self, universe):
sel = universe.select_atoms("resid 100:105")
assert_equal(sel.n_atoms, 89)
assert_equal(
sel.residues.resnames, ["GLY", "ILE", "ASN", "VAL", "ASP", "TYR"]
)
assert_equal(sel.residues.resnames, ["GLY", "ILE", "ASN", "VAL", "ASP", "TYR"])

def test_selgroup(self, universe):
sel = universe.select_atoms("not resid 100")
Expand All @@ -156,23 +155,15 @@ def test_resnum_range(self, universe):
sel = universe.select_atoms("resnum 100:105")
assert_equal(sel.n_atoms, 89)
assert_equal(sel.residues.resids, range(100, 106))
assert_equal(
sel.residues.resnames, ["GLY", "ILE", "ASN", "VAL", "ASP", "TYR"]
)
assert_equal(sel.residues.resnames, ["GLY", "ILE", "ASN", "VAL", "ASP", "TYR"])

def test_resname(self, universe):
sel = universe.select_atoms("resname LEU")
assert_equal(
sel.n_atoms, 304, "Failed to find all 'resname LEU' atoms."
)
assert_equal(
sel.n_residues, 16, "Failed to find all 'resname LEU' residues."
)
assert_equal(sel.n_atoms, 304, "Failed to find all 'resname LEU' atoms.")
assert_equal(sel.n_residues, 16, "Failed to find all 'resname LEU' residues.")
assert_equal(
sorted(sel.indices),
sorted(
universe.select_atoms("segid 4AKE and resname LEU").indices
),
sorted(universe.select_atoms("segid 4AKE and resname LEU").indices),
"selected 'resname LEU' atoms are not the same as auto-generated s4AKE.LEU",
)

Expand All @@ -186,9 +177,7 @@ def test_atom(self, universe):
assert_equal(sel.resnames, ["GLY"])
assert_equal(
sel.positions,
np.array(
[[20.38685226, -3.44224262, -5.92158318]], dtype=np.float32
),
np.array([[20.38685226, -3.44224262, -5.92158318]], dtype=np.float32),
)

def test_atom_empty(self, universe):
Expand Down Expand Up @@ -330,10 +319,7 @@ def test_same_resname(self, universe):
assert_equal(
len(sel),
331,
(
"Found a wrong number of atoms with same resname as "
"resids 10 or 11"
),
("Found a wrong number of atoms with same resname as " "resids 10 or 11"),
)
# fmt: off
target_resids = np.array(
Expand Down Expand Up @@ -428,9 +414,7 @@ def test_no_space_around_parentheses(self, universe):
def test_concatenated_selection(self, universe):
E151 = universe.select_atoms("segid 4AKE").select_atoms("resid 151")
# note that this is not quite phi... HN should be C of prec. residue
phi151 = E151.atoms.select_atoms(
"name HN", "name N", "name CA", "name CB"
)
phi151 = E151.atoms.select_atoms("name HN", "name N", "name CA", "name CB")
assert_equal(len(phi151), 4)
assert_equal(
phi151[0].name,
Expand All @@ -442,9 +426,7 @@ def test_global(self, universe):
"""Test the `global` modifier keyword (Issue 268)"""
ag = universe.select_atoms("resname LYS and name NZ")
# Lys amines within 4 angstrom of the backbone.
ag1 = universe.select_atoms(
"resname LYS and name NZ and around 4 backbone"
)
ag1 = universe.select_atoms("resname LYS and name NZ and around 4 backbone")
ag2 = ag.select_atoms("around 4 global backbone")
assert_equal(ag2.indices, ag1.indices)

Expand Down Expand Up @@ -495,12 +477,8 @@ def universe(self):
def test_protein(self, universe):
# must include non-standard residues
sel = universe.select_atoms("protein or resname HAO or resname ORT")
assert_equal(
sel.n_atoms, universe.atoms.n_atoms, "failed to select peptide"
)
assert_equal(
sel.n_residues, 6, "failed to select all peptide residues"
)
assert_equal(sel.n_atoms, universe.atoms.n_atoms, "failed to select peptide")
assert_equal(sel.n_residues, 6, "failed to select all peptide residues")

def test_resid_single(self, universe):
sel = universe.select_atoms("resid 12")
Expand Down Expand Up @@ -590,9 +568,7 @@ def test_same_fragment(self, universe, selstr):
# This test comes here because it's a system with solvent,
# and thus multiple fragments.
sel = universe.select_atoms(selstr)
errmsg = (
"Found a wrong number of atoms " "on the same fragment as id 1"
)
errmsg = "Found a wrong number of atoms " "on the same fragment as id 1"
assert_equal(len(sel), 3341, errmsg)
errmsg = (
"Found a differ set of atoms when using the 'same "
Expand Down Expand Up @@ -695,9 +671,7 @@ def test_passing_rdkit_kwargs_to_converter(self):
def test_passing_max_matches_to_converter(self, u2):
with pytest.warns(UserWarning, match="Your smarts-based") as wsmg:
sel = u2.select_atoms("smarts C", smarts_kwargs=dict(maxMatches=2))
sel2 = u2.select_atoms(
"smarts C", smarts_kwargs=dict(maxMatches=1000)
)
sel2 = u2.select_atoms("smarts C", smarts_kwargs=dict(maxMatches=1000))
assert sel.n_atoms == 2
assert sel2.n_atoms == 3

Expand Down Expand Up @@ -908,9 +882,7 @@ class TestOrthogonalDistanceSelections(BaseDistanceSelection):
def u(self):
return mda.Universe(TRZ_psf, TRZ)

@pytest.mark.parametrize(
"meth, periodic", [("distmat", True), ("distmat", False)]
)
@pytest.mark.parametrize("meth, periodic", [("distmat", True), ("distmat", False)])
def test_cyzone(self, u, meth, periodic):
sel = Parser.parse("cyzone 5 4 -4 resid 2", u.atoms)
sel.periodic = periodic
Expand Down Expand Up @@ -1178,9 +1150,7 @@ def test_flip(self, prop, ag, op):
# reference group, doing things forwards
ref = ag[func(getattr(ag, self.plurals[prop]), 1.5)]

selstr = "prop 1.5 {op} {prop}".format(
op=self.opposites[op], prop=prop
)
selstr = "prop 1.5 {op} {prop}".format(op=self.opposites[op], prop=prop)
sel = ag.select_atoms(selstr)

assert_equal(set(ref.indices), set(sel.indices))
Expand Down Expand Up @@ -1211,9 +1181,7 @@ class TestSelectionErrors(object):
@staticmethod
@pytest.fixture()
def universe():
return make_Universe(
("names", "masses", "resids", "resnames", "resnums")
)
return make_Universe(("names", "masses", "resids", "resnames", "resnums"))

@pytest.mark.parametrize(
"selstr",
Expand Down Expand Up @@ -1316,9 +1284,7 @@ def test_string_selections(self, ref, sel, universe):
],
)
def test_range_selections(self, seltype, ref, sel, universe):
self._check_sels(
ref.format(typ=seltype), sel.format(typ=seltype), universe
)
self._check_sels(ref.format(typ=seltype), sel.format(typ=seltype), universe)


class TestICodeSelection(object):
Expand Down Expand Up @@ -1662,9 +1628,7 @@ def test_bool_sel_error():

def test_error_selection_for_strange_dtype():
with pytest.raises(ValueError, match="No base class defined for dtype"):
MDAnalysis.core.selection.gen_selection_class(
"star", "stars", dict, "atom"
)
MDAnalysis.core.selection.gen_selection_class("star", "stars", dict, "atom")


@pytest.mark.parametrize(
Expand Down Expand Up @@ -1731,9 +1695,7 @@ def test_chirality(smi, chirality):
assert u.atoms[0].chirality == ""
assert u.atoms[1].chirality == chirality

assert_equal(
u.atoms[:3].chiralities, np.array(["", chirality, ""], dtype="U1")
)
assert_equal(u.atoms[:3].chiralities, np.array(["", chirality, ""], dtype="U1"))
Copy link
Member

@tylerjereddy tylerjereddy Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all these unrelated whitespace changes really needed?

Copy link
Author

@rishinamdev17 rishinamdev17 Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those whitespace changes are unrelated to my test. I'll revert them and keep the PR focused only on the cylayer test.



@pytest.mark.parametrize(
Expand Down Expand Up @@ -1774,3 +1736,20 @@ def test_formal_charge_selection(sel, size, name):

assert len(ag) == size
assert ag.atoms[0].name == name


@pytest.fixture(scope="module")
def universe():
u = mda.Universe(PSF, DCD)
u.dimensions = np.array([100.0, 100.0, 100.0, 90.0, 90.0, 90.0])
return u
Copy link
Member

@tylerjereddy tylerjereddy Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a fixture for use in a single test case seems a bit much, especially when you're repeating the setting of dimensions below anyway.

Copy link
Author

@rishinamdev17 rishinamdev17 Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have combined the universe setup into the test and removed the duplicate dimensions setting.



def test_cylayer_selection_parses_correctly(universe):
universe.dimensions = np.array([100, 100, 100, 90, 90, 90])

sel = universe.select_atoms("cylayer 5 10 15 -15 name CA")

# Basic sanity checks
assert len(sel) > 0
assert sel.n_atoms <= universe.atoms.n_atoms
Loading