-
Notifications
You must be signed in to change notification settings - Fork 700
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.
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
feature: IMDReader Integration #4923
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,152 @@ | ||||||
""" | ||||||
MDAnalysis IMDReader | ||||||
^^^^^^^^^^^^^^^^^^^^ | ||||||
|
||||||
.. autoclass:: IMDReader | ||||||
:members: | ||||||
:inherited-members: | ||||||
|
||||||
""" | ||||||
|
||||||
from MDAnalysis.coordinates import core | ||||||
from MDAnalysis.lib.util import store_init_arguments | ||||||
from MDAnalysis.coordinates.util import parse_host_port | ||||||
from MDAnalysis.coordinates.base import StreamReaderBase | ||||||
|
||||||
try: | ||||||
import imdclient | ||||||
from imdclient.IMDClient import IMDClient | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These need test coverage with mocks |
||||||
except ImportError: | ||||||
HAS_IMDCLIENT = False | ||||||
|
||||||
# Allow building doucmnetation without imdclient | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
import types | ||||||
|
||||||
class MockIMDClient: | ||||||
pass | ||||||
imdclient = types.ModuleType("imdclient") | ||||||
imdclient.IMDClient = MockIMDClient | ||||||
|
||||||
else: | ||||||
HAS_IMDCLIENT = True | ||||||
|
||||||
import logging | ||||||
|
||||||
logger = logging.getLogger("imdclient.IMDClient") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change to MDAnalysis.coordinates.IMDReader |
||||||
|
||||||
|
||||||
class IMDReader(StreamReaderBase): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm getting confused about which PR is for which thing. @orbeckst given our discussion earlier this week, and your comment above which I take to be "IMDClient is still in flux", does it not make sense for the IMDReader to exist upstream and then just import it here? (edit: here my intent is "well then you could make releases and it wouldn't be limited to MDA release frequency"). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have to split IMDReader from imdclient and make a version of imdclient without IMDReader (which is in the works Becksteinlab/imdclient#54 ). At the same time we are moving what was split off into coordinates.IMD. Amru is working on both at the moment. The way IMDReader depends on imdclient is not the problem, and imdclient itself is also pretty stable, it's just that the tests for imdclient have made use of a lot of MDAnalysis/IMDReader for convenience, and we now have to rewrite some of these tests to use bare-bones python. |
||||||
""" | ||||||
Reader for IMD protocol packets. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
filename : a string of the form "host:port" where host is the hostname | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this format checked on |
||||||
or IP address of the listening GROMACS server and port | ||||||
is the port number. | ||||||
n_atoms : int (optional) | ||||||
number of atoms in the system. defaults to number of atoms | ||||||
in the topology. don't set this unless you know what you're doing. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
kwargs : dict (optional) | ||||||
keyword arguments passed to the constructed :class:`IMDClient` | ||||||
""" | ||||||
|
||||||
format = "IMD" | ||||||
one_pass = True | ||||||
|
||||||
@store_init_arguments | ||||||
def __init__( | ||||||
self, | ||||||
filename, | ||||||
convert_units=True, | ||||||
n_atoms=None, | ||||||
**kwargs, | ||||||
): | ||||||
if not HAS_IMDCLIENT: | ||||||
raise ImportError( | ||||||
"IMDReader requires the imdclient package. " | ||||||
"Please install it with 'pip install imdclient'." | ||||||
) | ||||||
|
||||||
super(IMDReader, self).__init__(filename, **kwargs) | ||||||
|
||||||
self._imdclient = None | ||||||
logger.debug("IMDReader initializing") | ||||||
|
||||||
if n_atoms is None: | ||||||
raise ValueError("IMDReader: n_atoms must be specified") | ||||||
self.n_atoms = n_atoms | ||||||
|
||||||
host, port = parse_host_port(filename) | ||||||
|
||||||
# This starts the simulation | ||||||
self._imdclient = IMDClient(host, port, n_atoms, **kwargs) | ||||||
|
||||||
imdsinfo = self._imdclient.get_imdsessioninfo() | ||||||
# NOTE: after testing phase, fail out on IMDv2 | ||||||
|
||||||
self.ts = self._Timestep( | ||||||
self.n_atoms, | ||||||
positions=imdsinfo.positions, | ||||||
velocities=imdsinfo.velocities, | ||||||
forces=imdsinfo.forces, | ||||||
**self._ts_kwargs, | ||||||
) | ||||||
|
||||||
self._frame = -1 | ||||||
|
||||||
try: | ||||||
self._read_next_timestep() | ||||||
except StopIteration: | ||||||
raise RuntimeError("IMDReader: No data found in stream") | ||||||
|
||||||
def _read_frame(self, frame): | ||||||
|
||||||
try: | ||||||
imdf = self._imdclient.get_imdframe() | ||||||
except EOFError as e: | ||||||
raise e | ||||||
|
||||||
self._frame = frame | ||||||
self._load_imdframe_into_ts(imdf) | ||||||
|
||||||
logger.debug(f"IMDReader: Loaded frame {self._frame}") | ||||||
return self.ts | ||||||
|
||||||
def _load_imdframe_into_ts(self, imdf): | ||||||
self.ts.frame = self._frame | ||||||
if imdf.time is not None: | ||||||
self.ts.time = imdf.time | ||||||
# NOTE: timestep.pyx "dt" method is suspicious bc it uses "new" keyword for a float | ||||||
self.ts.data["dt"] = imdf.dt | ||||||
self.ts.data["step"] = imdf.step | ||||||
if imdf.energies is not None: | ||||||
self.ts.data.update( | ||||||
{k: v for k, v in imdf.energies.items() if k != "step"} | ||||||
) | ||||||
if imdf.box is not None: | ||||||
self.ts.dimensions = core.triclinic_box(*imdf.box) | ||||||
if imdf.positions is not None: | ||||||
# must call copy because reference is expected to reset | ||||||
# see 'test_frame_collect_all_same' in MDAnalysisTests.coordinates.base | ||||||
self.ts.positions = imdf.positions | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||||||
if imdf.velocities is not None: | ||||||
self.ts.velocities = imdf.velocities | ||||||
if imdf.forces is not None: | ||||||
self.ts.forces = imdf.forces | ||||||
|
||||||
@staticmethod | ||||||
def _format_hint(thing): | ||||||
try: | ||||||
parse_host_port(thing) | ||||||
except: | ||||||
return False | ||||||
return HAS_IMDCLIENT and True | ||||||
|
||||||
def close(self): | ||||||
"""Gracefully shut down the reader. Stops the producer thread.""" | ||||||
logger.debug("IMDReader close() called") | ||||||
if self._imdclient is not None: | ||||||
self._imdclient.stop() | ||||||
# NOTE: removeme after testing | ||||||
logger.debug("IMDReader shut down gracefully.") | ||||||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1841,3 +1841,194 @@ | |
|
||
def convert(self, obj): | ||
raise NotImplementedError | ||
|
||
class StreamReaderBase(ReaderBase): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep correct! |
||
|
||
def __init__(self, filename, convert_units=True, **kwargs): | ||
super(StreamReaderBase, self).__init__( | ||
filename, convert_units=convert_units, **kwargs | ||
) | ||
self._init_scope = True | ||
self._reopen_called = False | ||
self._first_ts = None | ||
|
||
def _read_next_timestep(self): | ||
# No rewinding- to both load the first frame after __init__ | ||
# and access it again during iteration, we need to store first ts in mem | ||
if not self._init_scope and self._frame == -1: | ||
self._frame += 1 | ||
# can't simply return the same ts again- transformations would be applied twice | ||
# instead, return the pre-transformed copy | ||
return self._first_ts | ||
|
||
ts = self._read_frame(self._frame + 1) | ||
|
||
if self._init_scope: | ||
self._first_ts = self.ts.copy() | ||
self._init_scope = False | ||
|
||
return ts | ||
|
||
@property | ||
def n_frames(self): | ||
"""Changes as stream is processed unlike other readers""" | ||
raise RuntimeError( | ||
"{}: n_frames is unknown".format(self.__class__.__name__) | ||
) | ||
|
||
def __len__(self): | ||
raise RuntimeError( | ||
"{} has unknown length".format(self.__class__.__name__) | ||
) | ||
|
||
def next(self): | ||
"""Don't rewind after iteration. When _reopen() is called, | ||
an error will be raised | ||
""" | ||
try: | ||
ts = self._read_next_timestep() | ||
except (EOFError, IOError): | ||
# Don't rewind here like we normally would | ||
raise StopIteration from None | ||
else: | ||
for auxname, reader in self._auxs.items(): | ||
ts = self._auxs[auxname].update_ts(ts) | ||
|
||
ts = self._apply_transformations(ts) | ||
|
||
return ts | ||
|
||
def rewind(self): | ||
"""Raise error on rewind""" | ||
raise RuntimeError( | ||
"{}: Stream-based readers can't be rewound".format( | ||
self.__class__.__name__ | ||
) | ||
) | ||
|
||
# Incompatible methods | ||
def copy(self): | ||
raise NotImplementedError( | ||
"{} does not support copying".format(self.__class__.__name__) | ||
) | ||
|
||
def _reopen(self): | ||
if self._reopen_called: | ||
raise RuntimeError( | ||
"{}: Cannot reopen stream".format(self.__class__.__name__) | ||
) | ||
self._frame = -1 | ||
self._reopen_called = True | ||
|
||
def __getitem__(self, frame): | ||
"""Return the Timestep corresponding to *frame*. | ||
|
||
If `frame` is a integer then the corresponding frame is | ||
returned. Negative numbers are counted from the end. | ||
|
||
If frame is a :class:`slice` then an iterator is returned that | ||
allows iteration over that part of the trajectory. | ||
|
||
Note | ||
---- | ||
*frame* is a 0-based frame index. | ||
""" | ||
if isinstance(frame, slice): | ||
_, _, step = self.check_slice_indices( | ||
frame.start, frame.stop, frame.step | ||
) | ||
if step is None: | ||
return FrameIteratorAll(self) | ||
else: | ||
return StreamFrameIteratorSliced(self, step) | ||
else: | ||
raise TypeError( | ||
"Streamed trajectories must be an indexed using a slice" | ||
) | ||
|
||
def check_slice_indices(self, start, stop, step): | ||
if start is not None: | ||
raise ValueError( | ||
"{}: Cannot expect a start index from a stream, 'start' must be None".format( | ||
self.__class__.__name__ | ||
) | ||
) | ||
if stop is not None: | ||
raise ValueError( | ||
"{}: Cannot expect a stop index from a stream, 'stop' must be None".format( | ||
self.__class__.__name__ | ||
) | ||
) | ||
if step is not None: | ||
if isinstance(step, numbers.Integral): | ||
if step < 1: | ||
raise ValueError( | ||
"{}: Cannot go backwards in a stream, 'step' must be > 0".format( | ||
self.__class__.__name__ | ||
) | ||
) | ||
else: | ||
raise ValueError( | ||
"{}: 'step' must be an integer".format( | ||
self.__class__.__name__ | ||
) | ||
) | ||
|
||
return start, stop, step | ||
|
||
def __getstate__(self): | ||
raise NotImplementedError( | ||
"{} does not support pickling".format(self.__class__.__name__) | ||
) | ||
|
||
def __setstate__(self, state: object): | ||
raise NotImplementedError( | ||
"{} does not support pickling".format(self.__class__.__name__) | ||
) | ||
|
||
def __repr__(self): | ||
return ( | ||
"<{cls} {fname} with continuous stream of {natoms} atoms>" | ||
"".format( | ||
cls=self.__class__.__name__, | ||
fname=self.filename, | ||
natoms=self.n_atoms, | ||
) | ||
) | ||
|
||
|
||
class StreamFrameIteratorSliced(FrameIteratorBase): | ||
|
||
def __init__(self, trajectory, step): | ||
super().__init__(trajectory) | ||
self._step = step | ||
|
||
def __iter__(self): | ||
# Calling reopen tells reader | ||
# it can't be reopened again | ||
self.trajectory._reopen() | ||
return self | ||
|
||
def __next__(self): | ||
try: | ||
# Burn the timesteps until we reach the desired step | ||
# Don't use next() to avoid unnecessary transformations | ||
while self.trajectory._frame + 1 % self.step != 0: | ||
self.trajectory._read_next_timestep() | ||
except (EOFError, IOError): | ||
# Don't rewind here like we normally would | ||
raise StopIteration from None | ||
|
||
return self.trajectory.next() | ||
|
||
def __len__(self): | ||
raise RuntimeError( | ||
"{} has unknown length".format(self.__class__.__name__) | ||
) | ||
|
||
def __getitem__(self, frame): | ||
raise RuntimeError("Sliced iterator does not support indexing") | ||
|
||
@property | ||
def step(self): | ||
return self._step | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's going to take time to get imdclient to a stage where the imdclient package does not actually affect MDAnalysis.
Is there a way that we could temporarily (for initial CI testing) install imdclient from a branch or tarball, e.g., in a
pip
section? Then we could fairly rapidly create a preliminary (unpublished) imdclient package without IMDReader.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By initial CI testing, do you mean "in this PR"?
There's a pip section just below, which should work if you put in the git location for pip install, but also you can just temporarily modify the CI script to do an additional pip install if it's for "testing within the PR itself".
If it's "after merge", this would require more discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for right now to bootstrap the PR.
I don't want to merge without a solid conda-forge imdclient package in place.