djay0529 / mdanalysis

Automatically exported from code.google.com/p/mdanalysis
0 stars 0 forks source link

trajectory writer interface is inconsistent #206

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
The interface to our trajectory writers is inconsistent and should be fixed.

- Not all of them accept the same options (or fail if one provides an 
unrecognized one), which is bad if one wants to write general code that can 
write out any format understood by MDAnalysis.

- The docs for MDAnalysis.coordinates.core. writer() (the dispatch function 
that selects a writer based on file extension and user input) says, for 
instance, that "some writers will have special arguments that have to be looked 
up" but also that there a number of common arguments are common with specific 
universal meanings for ALL writers. 
http://mdanalysis.googlecode.com/git-history/develop/package/doc/html/documentat
ion_pages/coordinates/core.html#MDAnalysis.coordinates.core.writer

- Specifically, "delta is length of time between two frames, in ps [1.0]". The 
DCDWriter, for instance, breaks this promise and instead expects delta to be 
the integrator timestep in AKMA units. The TRZWriter takes a clue from 
DCDWriter and also uses delta for integrator timestep. The Gromacs XTC/TRR 
writer on the other hand use delta as the time between saved frames in ps.

The Trajectory API 
http://mdanalysis.googlecode.com/git-history/develop/package/doc/html/documentat
ion_pages/coordinates/init.html#trajectory-api is actually vague on what 
Writers have to accept. This should be tightened and synced with the docs to 
writer().

At a minimum

- all writers need to treat start, stop, step, delta in the same manner
- all writers should gracefully deal with unknown parameters, i.e. implement 
catch-all **kwargs

Opinions?

Original issue reported on code.google.com by orbeckst on 20 Jan 2015 at 7:02

GoogleCodeExporter commented 9 years ago

Original comment by orbeckst on 20 Jan 2015 at 7:03

GoogleCodeExporter commented 9 years ago
The testing for Writers is pretty haphazard right now, which is arguably how 
they've ended up diverging.

After the next release I can write some tests like I did for Timesteps where 
there's a mixin test for Writer and then each format.

Timestep tests:
https://code.google.com/p/mdanalysis/source/browse/testsuite/MDAnalysisTests/tes
t_coordinates.py?name=develop#2778

Original comment by richardjgowers on 22 Jan 2015 at 4:00