Refactor all Transformations into class

[yuxuanzhuang]

Is your feature request related to a problem?

To make pickling transformation possible. Currently, due to the nature of python closure, Transformations cannot be pickled.

Describe the solution you'd like

Refactor those into class object, with __call__ function.

[yuxuanzhuang]

u = mda.Universe(TPR, XTC)
ts = u.trajectory.ts

transformations = []
transformations.append(translate([1,2,3]))
transformations.append(center_in_box(u.atoms))
transformations.append(rotateby(90, [0,0,1], ag=u.atoms))
transformations.append(PositionAverager(3))
transformations.append(fit_translation(u.atoms, u.atoms, 'xy', 'mass'))
transformations.append(fit_rot_trans(u.atoms, u.atoms, 'xy', 'mass'))
transformations.append(wrap(u.atoms))
transformations.append(unwrap(u.atoms))

for trans in transformations:
    trans_p = pickle.loads(pickle.dumps(trans))
    assert_equal(trans_p(ts), trans(ts))

[orbeckst]

This is breaking the transformations API and will also require rewriting docs in the User Guide so please raise an issue there once we're close to merging.

[jbarnoud]

It is unfortunate that we have to do this. I do understand why it is necessary, but using classes for what really should be functions add some complexity that I would have like to do without. While the change seems necessary for the transformations we provide, I would appreciate it if the documentation could keep showing how to use functions; not everything uses multiprocessing and needs pickling, and some may use parallelization stacks that can deal with closures (such, I think, as https://github.com/uqfoundation/pathos).

[yuxuanzhuang]

It is unfortunate that we have to do this. I do understand why it is necessary, but using classes for what really should be functions add some complexity that I would have like to do without. While the change seems necessary for the transformations we provide, I would appreciate it if the documentation could keep showing how to use functions; not everything uses multiprocessing and needs pickling, and some may use parallelization stacks that can deal with closures (such, I think, as https://github.com/uqfoundation/pathos).

If I understand correctly, pathos uses dill, which support serialization of nested functions. It is also the case for cloudpickle in dask.

So I guess it depends on the goal here, if we plan to stick to how pmda is implemented with dask, old transformations can already be used flawlessly in some cases. But if we plan to have a more generic case here, i.e. use multiprocessing of python, then I am afraid there's no way to (c)pickle a nested function (as far as I know).

[jbarnoud]

Sadly, there is indeed no way to use closures with multiprocessing. So the transformations sadly need to be converted. My point is mostly about the documentation. It will have to explain why transformations are classes, but it should keep saying that functions are OK as long as you do not plan to use pickle/multiprocessing.