Skip to content

Commit

Permalink
Backport fix from PR #281. [ci skip]
Browse files Browse the repository at this point in the history
  • Loading branch information
lohedges committed May 7, 2024
1 parent 791bbd4 commit a4664bc
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 26 deletions.
37 changes: 24 additions & 13 deletions python/BioSimSpace/Sandpit/Exscientia/Trajectory/_trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
# Update the water topology to match topology/trajectory.
system = _update_water_topology(system, topology, trajectory, property_map)

# Copy the system.
renumbered_system = system.copy()

# Make sure the constituents of the system are numbered in ascending order.
renumbered_system._sire_object = _SireIO.renumberConstituents(
system._sire_object
)

# Try to load the frame with Sire.
errors = []
is_sire = False
Expand Down Expand Up @@ -219,9 +227,7 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
if "space" in new_system.propertyKeys():
box = new_system.property("space")
if box.isPeriodic():
sire_system.setProperty(
self._property_map.get("space", "space"), box
)
sire_system.setProperty(property_map.get("space", "space"), box)

new_system = _System(sire_system)

Expand Down Expand Up @@ -265,7 +271,7 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
# coordinates of all of the atoms in the reference. As such, we
# will need to split the system into molecules.
new_system = _split_molecules(
frame, pdb, system, str(work_dir), property_map
frame, pdb, renumbered_system, str(work_dir), property_map
)
try:
sire_system, _ = _SireIO.updateCoordinatesAndVelocities(
Expand All @@ -276,9 +282,7 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
if "space" in new_system.propertyKeys():
box = new_system.property("space")
if box.isPeriodic():
sire_system.setProperty(
self._property_map.get("space", "space"), box
)
sire_system.setProperty(property_map.get("space", "space"), box)

new_system = _System(sire_system)
except Exception as e:
Expand All @@ -288,11 +292,6 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
else:
raise IOError(msg) from None

else:
raise IOError(
"The trajectory frame is incompatible with the passed system!"
)

# Load the frame directly to create a new System object.
else:
try:
Expand Down Expand Up @@ -474,6 +473,14 @@ def __init__(
self._system, self._top_file, self._traj_file, self._property_map
)

# Copy the system.
self._renumbered_system = self._system.copy()

# Make sure the constituents of the system are numbered in ascending order.
self._renumbered_system._sire_object = _SireIO.renumberConstituents(
self._system._sire_object
)

if not isinstance(backend, str):
raise TypeError("'backend' must be of type 'str'")

Expand Down Expand Up @@ -849,7 +856,7 @@ def getFrames(self, indices=None):
new_system = _split_molecules(
frame,
pdb,
self._system,
self._renumbered_system,
str(self._work_dir),
self._property_map,
)
Expand Down Expand Up @@ -1115,6 +1122,10 @@ def _split_molecules(frame, pdb, reference, work_dir, property_map={}):
# Store the formats associated with the reference system.
formats = reference.fileFormat()

# Convert NoneType to empty list.
if formats is None:
formats = []

# Write the frame coordinates/velocities to file.
coord_file = _os.path.join(str(work_dir), f"{str(_uuid.uuid4())}.coords")
top_file = _os.path.join(str(work_dir), f"{str(_uuid.uuid4())}.top")
Expand Down
37 changes: 24 additions & 13 deletions python/BioSimSpace/Trajectory/_trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
# Update the water topology to match topology/trajectory.
system = _update_water_topology(system, topology, trajectory, property_map)

# Copy the system.
renumbered_system = system.copy()

# Make sure the constituents of the system are numbered in ascending order.
renumbered_system._sire_object = _SireIO.renumberConstituents(
system._sire_object
)

# Try to load the frame with Sire.
errors = []
is_sire = False
Expand Down Expand Up @@ -219,9 +227,7 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
if "space" in new_system.propertyKeys():
box = new_system.property("space")
if box.isPeriodic():
sire_system.setProperty(
self._property_map.get("space", "space"), box
)
sire_system.setProperty(property_map.get("space", "space"), box)

new_system = _System(sire_system)

Expand Down Expand Up @@ -265,7 +271,7 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
# coordinates of all of the atoms in the reference. As such, we
# will need to split the system into molecules.
new_system = _split_molecules(
frame, pdb, system, str(work_dir), property_map
frame, pdb, renumbered_system, str(work_dir), property_map
)
try:
sire_system, _ = _SireIO.updateCoordinatesAndVelocities(
Expand All @@ -276,9 +282,7 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
if "space" in new_system.propertyKeys():
box = new_system.property("space")
if box.isPeriodic():
sire_system.setProperty(
self._property_map.get("space", "space"), box
)
sire_system.setProperty(property_map.get("space", "space"), box)

new_system = _System(sire_system)
except Exception as e:
Expand All @@ -288,11 +292,6 @@ def getFrame(trajectory, topology, index, system=None, property_map={}):
else:
raise IOError(msg) from None

else:
raise IOError(
"The trajectory frame is incompatible with the passed system!"
)

# Load the frame directly to create a new System object.
else:
try:
Expand Down Expand Up @@ -474,6 +473,14 @@ def __init__(
self._system, self._top_file, self._traj_file, self._property_map
)

# Copy the system.
self._renumbered_system = self._system.copy()

# Make sure the constituents of the system are numbered in ascending order.
self._renumbered_system._sire_object = _SireIO.renumberConstituents(
self._system._sire_object
)

if not isinstance(backend, str):
raise TypeError("'backend' must be of type 'str'")

Expand Down Expand Up @@ -849,7 +856,7 @@ def getFrames(self, indices=None):
new_system = _split_molecules(
frame,
pdb,
self._system,
self._renumbered_system,
str(self._work_dir),
self._property_map,
)
Expand Down Expand Up @@ -1115,6 +1122,10 @@ def _split_molecules(frame, pdb, reference, work_dir, property_map={}):
# Store the formats associated with the reference system.
formats = reference.fileFormat()

# Convert NoneType to empty list.
if formats is None:
formats = []

# Write the frame coordinates/velocities to file.
coord_file = _os.path.join(str(work_dir), f"{str(_uuid.uuid4())}.coords")
top_file = _os.path.join(str(work_dir), f"{str(_uuid.uuid4())}.top")
Expand Down
21 changes: 21 additions & 0 deletions tests/Sandpit/Exscientia/Trajectory/test_trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,24 @@ def test_rmsd(traj_sire, traj_mdtraj, traj_mdanalysis):
for v0, v1, v2 in zip(rmsd0, rmsd1, rmsd2):
assert v0.value() == pytest.approx(v1.value(), abs=1e-2)
assert v0.value() == pytest.approx(v2.value(), abs=1e-2)


@pytest.mark.skipif(has_mdtraj is False, reason="Requires mdtraj to be installed.")
def test_getFrame(system):
"""Regression test to make sure the getFrame function works."""

# Just load the trajectory directly.
frame = BSS.Trajectory.getFrame(
"tests/input/ala.trr", topology="tests/input/ala.gro", index=5
)

assert frame.nAtoms() == system.nAtoms()

# Load using the system as a reference.
frame = BSS.Trajectory.getFrame(
"tests/input/ala.trr", "tests/input/ala.gro", system=system, index=5
)

assert frame.nMolecules() == system.nMolecules()
assert frame.nResidues() == system.nResidues()
assert frame.nAtoms() == system.nAtoms()
21 changes: 21 additions & 0 deletions tests/Trajectory/test_trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,24 @@ def test_rmsd(traj_sire, traj_mdtraj, traj_mdanalysis):
for v0, v1, v2 in zip(rmsd0, rmsd1, rmsd2):
assert v0.value() == pytest.approx(v1.value(), abs=1e-2)
assert v0.value() == pytest.approx(v2.value(), abs=1e-2)


@pytest.mark.skipif(has_mdtraj is False, reason="Requires mdtraj to be installed.")
def test_getFrame(system):
"""Regression test to make sure the getFrame function works."""

# Just load the trajectory directly.
frame = BSS.Trajectory.getFrame(
"tests/input/ala.trr", topology="tests/input/ala.gro", index=5
)

assert frame.nAtoms() == system.nAtoms()

# Load using the system as a reference.
frame = BSS.Trajectory.getFrame(
"tests/input/ala.trr", "tests/input/ala.gro", system=system, index=5
)

assert frame.nMolecules() == system.nMolecules()
assert frame.nResidues() == system.nResidues()
assert frame.nAtoms() == system.nAtoms()

0 comments on commit a4664bc

Please sign in to comment.