Skip to content
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

Remove use of trajectory.timeseries from ENCORE #3761

Closed
hmacdope opened this issue Jul 23, 2022 · 2 comments · Fixed by #3890
Closed

Remove use of trajectory.timeseries from ENCORE #3761

hmacdope opened this issue Jul 23, 2022 · 2 comments · Fixed by #3890

Comments

@hmacdope
Copy link
Member

Expected behavior

The ENCORE module should work with all kinds of readers. As revealed by the use of the DUMPReader in this mailing list question this is not the case.

Actual behavior

The use of trajectory.timestep in ENCORE in many places means it cannot use readers that do not have the timeseries attribute. Currently only DCDReader and MemoryReader expose the timeseries attribute in the correct way to allow ENCORE to work.

Code to reproduce the behavior

import MDAnalysis as mda
from MDAnalysis.analysis import encore
from MDAnalysisTests.datafiles import LAMMPSDUMP
u = mda.Universe(LAMMPSDUMP, format='LAMMPSDUMP',atom_style="id type x y z")

es_conv = encore.ces_convergence(u,  50,  select='index 1') 

The current workaround is to load your trajectory into the MemoryReader using AnalysisFromFunction

import MDAnalysis as mda
from MDAnalysis.coordinates.memory import MemoryReader
from MDAnalysis.analysis.base import AnalysisFromFunction
from MDAnalysisTests.datafiles import PDB, XTC
from MDAnalysis.analysis import encore

u = mda.Universe(PDB, XTC)
coordinates = AnalysisFromFunction(lambda ag: ag.positions.copy(), u.atoms).run().results
u2 = mda.Universe(PDB, coordinates['timeseries'], format=MemoryReader)
es_conv = encore.ces_convergence(u2,  50, select='index 1')

Solution

We should change to using AtomGroup.positions.

@IAlibay
Copy link
Member

IAlibay commented Jul 23, 2022

related MDAnalysis/mdaencore#30 #2948

@hmacdope
Copy link
Member Author

hmacdope commented Nov 6, 2022

Instead we have implemented timeseries in ReaderBase in #3890 so all readers will now have access to it which should solve this issue the reverse way.

@hmacdope hmacdope linked a pull request Nov 6, 2022 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants