diff --git a/package/CHANGELOG b/package/CHANGELOG index a832609b39a..71411414874 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -23,6 +23,7 @@ The rules for this file: * 2.11.0 Fixes + * Fix FileLock tests for XTC and TRR: lock file is no longer removed (#5382) * InterRDF now correctly returns bins in parallel (PR #5344) * `Merge()` no longer raises a TypeError on Universes that have a `cmaps` attribute; cmaps are now combined like the other connection attributes diff --git a/package/MDAnalysis/coordinates/XDR.py b/package/MDAnalysis/coordinates/XDR.py index feb65c42882..fa30362b91a 100644 --- a/package/MDAnalysis/coordinates/XDR.py +++ b/package/MDAnalysis/coordinates/XDR.py @@ -32,13 +32,42 @@ MDAnalysis.coordinates.XTC: Read and write GROMACS XTC trajectory files. MDAnalysis.coordinates.TRR: Read and write GROMACS TRR trajectory files. MDAnalysis.lib.formats.libmdaxdr: Low level xdr format reader + + +XDR reader class +---------------- + +The :class:`XDRBaseReader` contains common functionality for the TRR and XTC +reader for GROMACS files, which are implemented in the +:mod:`MDAnalysis.lib.formats.libmdaxdr` module. + +Both formats have in common that they do not allow +native random frame access. Therefore, we first scan the whole trajectory to +build an index of frames in the file ("offsets") as a look-up for seeking to +frames. This process is initially slow so we save the offsets to a hidden file +next to the trajectory (if possible) and then read the offset file when the +trajectory is opened the next time, as described under :ref:`Offsets`. + +.. autoclass:: XDRBaseReader + :members: + :inherited-members: + :private-members: + + +Functions +--------- + +.. autofunction:: offsets_filename + +.. autofunction:: read_numpy_offsets + """ import errno import numpy as np from os.path import getctime, getsize, isfile, split, join import warnings -from filelock import FileLock +import filelock from . import base from ..lib.mdamath import triclinic_box @@ -95,8 +124,8 @@ class XDRBaseReader(base.ReaderBase): """Base class for libmdaxdr file formats xtc and trr This class handles integration of XDR based formats into MDAnalysis. The - XTC and TRR classes only implement `_write_next_frame` and - `_frame_to_ts`. + XTC and TRR classes only implement :meth:`_write_next_frame` and + :meth:`_frame_to_ts`. .. _offsets-label: @@ -104,18 +133,21 @@ class XDRBaseReader(base.ReaderBase): ----- XDR based readers store persistent offsets on disk. The offsets are used to enable access to random frames efficiently. These offsets will be generated - automatically the first time the trajectory is opened. Generally offsets - are stored in hidden `*_offsets.npz` files. Afterwards opening the same + automatically the first time the trajectory is opened. Generally offsets + are stored in hidden ``*_offsets.npz`` files. Afterwards opening the same file again is fast. It sometimes can happen that the stored offsets get out off sync with the trajectory they refer to. For this the offsets also store the number of atoms, size of the file and last modification time. If any of - them change the offsets are recalculated. Writing of the offset file can - fail when the directory where the trajectory file resides is not writable - or if the disk is full. In this case a warning message will be shown but + them change the offsets are recalculated. Writing of the offset file can + fail when the directory where the trajectory file resides is not writable + or if the disk is full. In this case a warning message will be shown but the offsets will nevertheless be used during the lifetime of the trajectory - Reader. However, the next time the trajectory is opened, the offsets will + Reader. However, the next time the trajectory is opened, the offsets will have to be rebuilt again. + See :meth:`_load_offsets` for further details. + + .. versionchanged:: 1.0.0 XDR offsets read from trajectory if offsets file read-in fails .. versionchanged:: 2.0.0 @@ -124,6 +156,7 @@ class XDRBaseReader(base.ReaderBase): Use a direct read into ts attributes .. versionchanged:: 2.9.0 Changed fasteners.InterProcessLock() to filelock.FileLock + """ @store_init_arguments @@ -204,15 +237,52 @@ def close(self): self._xdr.close() def _load_offsets(self): - """load frame offsets from file, reread them from the trajectory if that - fails. To prevent the competition of generating the same offset file - from multiple processes, an `InterProcessLock` is used.""" + """load frame offsets from file or recalculate if necessary + + Frame offsets are cached in an offsets file, which is stored as a + hidden file in the same directory as the trajectory. If the file does + not exist we generate the offsets and store them. + + If the data in the offset file are outdated (older than the trajectory + file or different number of frames from the trajectory or different + file size) then the offset file is also regenerated. + + .. Note:: + + Generating offsets can take minutes for large trajectories because + the whole file must be scanned. During this time, code appears to + hang. + + You can force regenerating offsets with the `refresh_offsets` keyword + argument for :class:`~MDAnalysis.core.universe.Universe`, for + example,:: + + u = mda.Universe(TOPOLOGY, XTC, refresh_offsets=True) + + To prevent the competition of generating the same offset file from + multiple processes, a :attr:`filelock.FileLock` is used, which is + implemented via a lock file (in the same directory as the offset file + and ending in ".lock"). This lock file is *not* automatically deleted + because doing so could lead to race conditions. + + Once this method completes, the + :attr:`~MDAnalysis.lib.formats.libmdaxdr.XTCFile.offsets` attribute of + the underlying reader contains current offsets for the trajectory. + + + .. SeeAlso:: + - :func:`offsets_filename` + - :meth:`_read_offsets` + - :func:`read_numpy_offsets` + + """ fname = offsets_filename(self.filename) lock_name = offsets_filename(self.filename, ending="lock") # check if the location of the lock is writable. + lock = filelock.FileLock(lock_name) try: - with FileLock(lock_name) as filelock: + with lock: pass except OSError as e: if isinstance(e, PermissionError) or e.errno == errno.EROFS: @@ -225,7 +295,7 @@ def _load_offsets(self): else: raise - with FileLock(lock_name) as filelock: + with lock: if not isfile(fname): self._read_offsets(store=True) return @@ -267,7 +337,24 @@ def _load_offsets(self): self._xdr.set_offsets(data["offsets"]) def _read_offsets(self, store=False): - """read frame offsets from trajectory""" + """read frame offsets from trajectory + + Scan the trajectory for frames and build an index that relates frame + number to the position in the file, thus enabling direct seeking to + specific frames. The trajectory scan can take minutes for large + trajectories. + + Parameters + ---------- + store : bool + Save the frame index ("offsets") to a file with name generated from + the trajectory name (:attr:`filename`) with function + :func:`offsets_filename`. The offsets file also contains, ctime, + file size, number of frames, and number of atoms of the trajectory. + The file format is a compressed numpy array (:func:`numpy.savez`). + + If saving the file fails for any reasons, only a warning is issued. + """ fname = offsets_filename(self.filename) offsets = self._xdr.offsets if store: diff --git a/package/doc/sphinx/source/conf.py b/package/doc/sphinx/source/conf.py index 44070577893..3309e3115bd 100644 --- a/package/doc/sphinx/source/conf.py +++ b/package/doc/sphinx/source/conf.py @@ -3,7 +3,8 @@ # MDAnalysis documentation build configuration file, created by # sphinx-quickstart on Mon Sep 27 09:39:55 2010. # -# This file is execfile()d with the current directory set to its containing dir. +# This file is execfile()d with the current directory set to its containing +# dir. # # Note that not all possible configuration values are present in this # autogenerated file. @@ -29,13 +30,13 @@ # make sure sphinx always uses the current branch sys.path.insert(0, os.path.abspath("../../..")) -# -- General configuration ----------------------------------------------------- +# -- General configuration ---------------------------------------------------- # If your documentation needs a minimal Sphinx version, state it here. # needs_sphinx = '1.0' -# Add any Sphinx extension module names here, as strings. They can be extensions -# coming with Sphinx (named 'sphinx.ext.*') or your custom ones. +# Add any Sphinx extension module names here, as strings. They can be +# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom ones. extensions = [ "sphinx.ext.autodoc", "sphinx.ext.intersphinx", @@ -70,7 +71,8 @@ class KeyStyle(UnsrtStyle): register_plugin("pybtex.style.labels", "keylabel", KeyLabelStyle) register_plugin("pybtex.style.formatting", "MDA", KeyStyle) -mathjax_path = "https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.0/MathJax.js?config=TeX-AMS-MML_HTMLorMML" + +mathjax_path = "https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.0/MathJax.js?config=TeX-AMS-MML_HTMLorMML" # noqa: E501; fmt: skip # for sitemap with https://github.com/jdillard/sphinx-sitemap # This sitemap is correct both for the development and release docs, which @@ -96,8 +98,8 @@ class KeyStyle(UnsrtStyle): # General information about the project. # (take the list from AUTHORS) -# Ordering: (1) Naveen (2) Elizabeth, then all contributors in alphabetical order -# (last) Oliver +# Ordering: (1) Naveen (2) Elizabeth, then all contributors in alphabetical +# order (last) Oliver author_list = mda.__authors__ authors = ", ".join(author_list[:-1]) + ", and " + author_list[-1] project = "MDAnalysis" @@ -130,7 +132,8 @@ class KeyStyle(UnsrtStyle): # directories to ignore when looking for source files. exclude_patterns = ["_build"] -# The reST default role (used for this markup: `text`) to use for all documents. +# The reST default role (used for this markup: `text`) to use for all +# documents. # default_role = None # If true, '()' will be appended to :func: etc. cross-reference text. @@ -156,7 +159,7 @@ class KeyStyle(UnsrtStyle): # to prevent including of member entries in toctree toc_object_entries = False -# -- Options for HTML output --------------------------------------------------- +# -- Options for HTML output -------------------------------------------------- # The theme to use for HTML and HTML Help pages. See the documentation for # a list of builtin themes. @@ -246,7 +249,7 @@ class KeyStyle(UnsrtStyle): htmlhelp_basename = "MDAnalysisdoc" -# -- Options for LaTeX output -------------------------------------------------- +# -- Options for LaTeX output ------------------------------------------------- # The paper size ('letter' or 'a4'). # latex_paper_size = 'letter' @@ -255,7 +258,8 @@ class KeyStyle(UnsrtStyle): # latex_font_size = '10pt' # Grouping the document tree into LaTeX files. List of tuples -# (source start file, target name, title, author, documentclass [howto/manual]). +# (source start file, target name, title, author, documentclass +# [howto/manual]). latex_documents = [ ("MDAnalysis.tex", "MDAnalysis Documentation", authors, "manual"), ] @@ -284,14 +288,14 @@ class KeyStyle(UnsrtStyle): # latex_domain_indices = True -# -- Options for manual page output -------------------------------------------- +# -- Options for manual page output ------------------------------------------- # One entry per manual page. List of tuples # (source start file, name, description, authors, manual section). man_pages = [("mdanalysis", "MDAnalysis Documentation", [authors], 1)] -# -- Options for Epub output --------------------------------------------------- +# -- Options for Epub output -------------------------------------------------- # Bibliographic Dublin Core info. epub_title = "MDAnalysis" @@ -352,4 +356,5 @@ class KeyStyle(UnsrtStyle): "imdclient": ("https://imdclient.readthedocs.io/en/stable/", None), "pooch": ("https://www.fatiando.org/pooch/latest/", None), "requests": ("https://requests.readthedocs.io/en/latest/", None), + "filelock": ("https://py-filelock.readthedocs.io/en/latest/", None), } diff --git a/package/doc/sphinx/source/documentation_pages/coordinates/XDR.rst b/package/doc/sphinx/source/documentation_pages/coordinates/XDR.rst index 1c585e324ef..73f7e92e9c8 100644 --- a/package/doc/sphinx/source/documentation_pages/coordinates/XDR.rst +++ b/package/doc/sphinx/source/documentation_pages/coordinates/XDR.rst @@ -1,3 +1 @@ .. automodule:: MDAnalysis.coordinates.XDR - :members: - :inherited-members: diff --git a/testsuite/MDAnalysisTests/coordinates/test_xdr.py b/testsuite/MDAnalysisTests/coordinates/test_xdr.py index c23fbf47da4..0af2433bb68 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_xdr.py +++ b/testsuite/MDAnalysisTests/coordinates/test_xdr.py @@ -22,12 +22,13 @@ # import pytest from unittest.mock import patch +from packaging.version import Version import re import os import shutil import sys -from filelock import FileLock +import filelock from pathlib import Path import numpy as np @@ -1028,7 +1029,7 @@ def test_persistent_offsets_readonly(self, tmpdir, trajectory): ref_offset = trajectory._xdr.offsets # Mock filelock acquire to raise an error with patch.object( - FileLock, "acquire", side_effect=PermissionError + filelock.FileLock, "acquire", side_effect=PermissionError ): # Simulate failure with pytest.warns(UserWarning, match="Cannot write lock"): reader = self._reader(filename) @@ -1048,21 +1049,32 @@ def test_persistent_offsets_readonly(self, tmpdir, trajectory): False, ) + @pytest.mark.xfail( + Version(filelock.__version__) < Version("3.29.5"), + reason="unsecure version of filelock", + ) def test_offset_lock_created(self): lock_file_path = XDR.offsets_filename(self.filename, ending="lock") - with FileLock(lock_file_path) as lock: + with filelock.FileLock(lock_file_path) as lock: # Lock acquired in context manager, so lock file should exist assert lock.is_locked assert os.path.exists(lock_file_path) - # Explicitly release lock, file should be deleted on UNIX - lock.release() - assert not lock.is_locked - if not sys.platform.startswith("win"): - # As of filelock>=3.21.0, filelock explicitly deletes lockfile - # upon release on UNIX. filelock does not do that on windows. - assert not os.path.exists(lock_file_path) + # released lock + assert not lock.is_locked + + # validate the expected behavior of how filelock handles lock files for + # released locks: + if sys.platform.startswith("win"): + # on Windows, the lockfile remained (filelock ~3.21.0) but recent + # versions (at least 3.29.7) appear to remove the lockfile + assert not os.path.exists(lock_file_path) + else: + # for filelock>=3.21.0,<3.29.5, the lockfile was deleted on POSIX, + # but this can lead to race conditions. Secure versions of filelock + # keep the lockfile (see GH tox-dev/filelock#574) + assert os.path.exists(lock_file_path) class TestXTCReader_offsets(_GromacsReader_offsets):