DCDReader units are not handled consistently

[orbeckst]

tl;dr: Manipulating the "native units" of the different DCDReader flavors (CHARMM, LAMMPS, soon NAMD) is probably still buggy and needs a few eyes.

The way we can force different units for the DCDReader(s) is still murky. PR #472 addresses some problems (in particular, it actually enables setting dt as kwarg). If one wants to change the unit of time or length one uses lengthunit or timeunit kwargs (to Universe, which passes through to the DCDReader). The native units of the reader are stored in DCDReader.units.

For the LAMMPS flavor of DCDReader it is common to change the units as a user (see units style command) so there's code to change LAMMPS.DCDReader.units. For the CHARMM flavor we don't do that (and units is class level, so it would be a bad idea to change it).

• Should we put all DCDReaders on the same footing and make native units changeable with kwargs? Or should we say that "CHARMM DCD" is defined to be AKMA/Å units and if you need something else you need to use a less stringent reader (e.g. LAMMPS or NAMD)?
• Do units properly propagate through the 'OtherWriter()' method that is supposed to give a writer that produces a copy of the trajectory? Should it use original native units (and dt) or propagate the user-set length- and time-unit and dt?

I want to introduce a NAMD flavor #359 as well, and, because the underlying DCDReader C-code is actually a NAMD reader, this would become the "base" implementation of DCDReader, with other flavors being derived from it.

[orbeckst]

I think the CHARMM DCDReader/DCDWriter should use fixed units because this is how CHARMM handles trajectories:

• time: AKMA
• length: Å

(An issue remains how we actually store unit cells in the DCD file (see #187); for as long as we are using the catdcd DCD code, we will be storing it in NAMD format – #359).

[richardjgowers]

So I don't fully understand all the permutations of DCD.. but what seems to make sense to me is something like...

class DCDReader:
    default_units = {}  # default units for DCDReader
    def __init__(self, **kwargs):
        # the units for this instance of Reader are from kwargs, or use the class defaults
        self.units = self.units.update(kwargs)

class CHARMMDCDReader:
    default_units = {AKMA & A}

I think this achieves both the scenarios in your first point, at the cost of having a few extra Classes.

With fetching Writers, it makes sense for a new Writer to have the default units of the class, unless kwargs say otherwise. But maybe as a convenience, some sort of same_units kwarg to get a Writer with the same units.

[orbeckst]

@richardjgowers's suggestion sounds good. Maybe I make a DCD subdirectory with the different flavors.

(I added it to 0.13 because it should be addressed together with #359 but if might get retargeted...)

[orbeckst]

In some way the DCD cython re-implementation #1372 made this issue superfluous: we established that CHARMM and NAMD DCDs have the same units. They do differ by unitcell representation but that's a different issue.