Error loading XDR files with corrupted offset files

[yuxuanzhuang]

Expected behavior

The offset file should be regenerated when the offset is corrupted.

Actual behavior

Unclear ValueError is raised.

~/anaconda3/envs/yzhuang/lib/python3.8/site-packages/numpy/lib/npyio.py in load(file, mmap_mode, allow_pickle, fix_imports, encoding)
    442             # Try a pickle
    443             if not allow_pickle:
--> 444                 raise ValueError("Cannot load file containing pickled data "
    445                                  "when allow_pickle=False")
    446             try:

ValueError: Cannot load file containing pickled data when allow_pickle=False

Code to reproduce the behavior

#/usr/bin/bash
# generate an empty npz file
touch .md.xtc_offsets.npz

import MDAnalysis as mda

u = mda.Universe('./md.pdb', './md.xtc')
....

Possible solution

Add an ValueError exception catch for read_numpy_offsets.

Note

This happened when I tried to process ~100 different trajectories parallelly, but it may not be related to #1988. Might just because I killed the same job beforehand and the offset files ended up corrupted. I will update if that's the case.

Current version of MDAnalysis

• Which version are you using? (run python -c "import MDAnalysis as mda; print(mda.__version__)") 2.0.0-dev
• Which version of Python (python -V)? 3.8
• Which operating system? Ubuntu

[hejung]

I ran into the same issue recently when processing trajectories using multiprocessing, i.e. when accessing the offsets file at the same time from two different processes. Passing refresh_offsets=True to the universe creation solves the issue, but it took me a while to figure out what the underlying issue is, so I think it would be nice for other users if something is done about it.

If wanted I can submit a PR in which the ValueError from numpy is caught, but I am not sure if that is the right thing to do. (One could take the stance that it is wrong to raise a ValueError when the file can not be read and it should therefore be fixed in numpy.load). Please let me know if I should submit a PR (and maybe also which branch to target with the PR).

[orbeckst]

@hejung PRs should be raised against develop.

It's easier to talk about code in a PR so if you have an initial draft of a idea, make a PR and then we can figure out if that's a good solution.

My feeling is that the offset file mechanism is fundamentally problematic in a multiprocessing context. It might need a lock file or some other mechanism to avoid race conditions. Maybe @mnmelo has some opinions on the matter?

[yuxuanzhuang]

I am trying to utilize shared_memory(https://docs.python.org/3/library/multiprocessing.shared_memory.html) to create a global flag indicating whether the offset file is being created or not. My hunch is that it is better to be some internal mechanism that handles the race condition automatically instead of adding any extra lines in the parallel code. I will most likely create a PR for that next week.

[hejung]

I would have simply changed this line from

except IOError:

to

except IOError, ValueError:

The reason beeing that np.load() as a last resort tries to load the .npz file with pickle which is only allowed if allow_pickle=True. If allow_pickle=False it will raise the (arguably wrong) ValueError about allow_pickle. The relevant lines in the numpy.load source are starting here.

This would solve the issues with corrupted offset files and the multiprocessing case by resorting to loading the offsets from the trajectory, but has the caveat of potentially catching other unrelated ValueErrors (which is why I was unsure if it is the right thing to do).

[orbeckst]

Submit a PR and we can discuss the merits there. One question that will surely come up is how to test the situation.