Background

"Snapshot features" are a really powerful tool in OPS that make it easy to create a snapshot class based on the fields that should be associated with that snapshot. Those fields vary from engine to engine.

Unfortunately, the function that handles this, the attach_features decorator, is written in a way that makes it extremely hard to maintain or test. It is a single function taking over 500 lines in the file, which has a CodeClimate cognitive complexity of 203. (For context, CodeClimate recommends keeping that number under 5, and I find that anything over 10 or 15 can be a headache to deal with.)

Additionally, the documentation for snapshot features is not very good. The best docs are in the docstring for the _decorator method nested inside attach_features, which can't be viewed unless you actually read the file openpathsampling/engines/features/base.py. This is not easy to find, and even that docstring doesn't quite fully correspond to what the code does.

For these reasons, I want to reimplement the attach_features decorator, and simplify some the process of creating snapshot features.

A big part of the functionality of attach_features is that it actually writes the code for several methods on the snapshot, including: copy, copy_to, create_reversed, create_empty, __init__, and init_empty. The reason to write code here, instead of just using the information in the __features__ object, is to get the correct signature and docstring for introspection. For example, we can do:

In [1]: import openpathsampling as paths

In [2]: paths.engines.toy.Snapshot?
Init signature: paths.engines.toy.Snapshot(velocities=None, coordinates=None, engine=None)
Docstring:
Simulation snapshot. Only references to coordinates and velocities

Attributes
----------
velocities : numpy.ndarray, shape=(atoms, 3), dtype=numpy.float32
    atomic velocities

coordinates : numpy.ndarray, shape=(atoms, 3), dtype=numpy.float32
    atomic coordinates

engine : :class:`openpathsampling.DynamicsEngine`
    reference to the engine used to generate the snapshot

where the signature and docstrings are determined by the features that are attached to the snapshot.

Changes in this PR

The goal here is to simplify both the process of creating new snapshot features and the code for attaching snapshot features, as well as to clean up some of the code that was autogenerated.

New FeatureCollection class

The current implementation uses a FeatureTuple namedtuple which is populated by the attach_features decorator. This has been replaced with a FeatureCollection class. In general the approach is to create a FeatureCollection for each feature module using its from_module method, and then to combine all the FeatureCollections into one FeatureCollection by summing them.

Simpler attach_features decorator

This replaces the old attach_features decorator with a new one, which has considerably simpler code structure. First, all the features are gathered into a single FeatureCollection, then the input class is decorated by adding the necessary functions, etc.

Separate code writing

Rather than including all the code for code writing/compiling in the attach_features method, this is put in a separate module, which should improve testability.

API break: Removing copy, copy_to

Snapshots are immutable data-containing objects. I do not think there is a need for the copy or copy_to methods. There is, however, a copy_with_replacement method -- if you give it no arguments, it will create an exact copy of your snapshot with a different UUID. It also provides the ability to create modified version of snapshots.

API break: Removing init_empty, fixing create_empty

I don't see a purpose for init_empty that isn't already handled by create_empty (again, with the understanding that these are immutable objects). create_empty might be used as a template or a placeholder, so I'm keeping it around, but the code it currently generates is wrong: it should be a classmethod, but it isn't marked as one, and it uses self in the signature but cls in the code.

Moving more functionality into superclass methods (new Snapshot base class)

Rather than writing out self-contained methods for each class, the new code keeps the core logic in a new Snapshot superclass, and simply passes the parameters on when needed. The key observation here is that we need dynamically-generated code for the purpose of signatures and docstrings, but the logic can be in general functions. This should be better for testing and for debugging.

New Parameter class to define field for a snapshot

In the old approach, you needed to go through the following steps to define a snapshot field (e.g., coordinates or velocities)

1. Add the name to the variables list in the feature module
2. Decide how to change that feature on time reversal: if it was a bool that flipped, add it to the list named flip in the module. If it was a number that changed sign, add it to the list named minus in the feature.
3. If it is a numpy array, add it to the list named numpy in the feature.
4. If it will be loaded by a lazy proxy, add it to the list named lazy in the feature.
5. Add a docstring to the module file, covering all the variables included.

In the new approach, we have a Parameter class that will contain the relevant information. With the old way, you may have needed to consider several top-level names for a given functionality (does this parameter go in the flip list for time reversal? in the minus list? neither? certainly not both, that would be nonsense -- but you could write code that tried that!) The new approach makes a class that describes each parameter, and the specific types of functionality (what to do on time reversal; how to copy; whether to lazy load) are contained in that object.

I think this is an easier way to think about these things, and it also has the advantage that instead of predefining certain behaviors (e.g., for copying), developers are welcome to create their own.

Time-reversal is treated a little differently than copying. Copying is an issue in computer programming, and is broadly relevant. Time-reversal is specific to MD. It's entirely possible that other such specific problems will arise in some subfield, so I tried to design a generic solution for them. Therefore, 'time_reverse' is an entry in a dict called operations. The operations can be extended with other domain-specific needs as they are found.

Decorator to declare instance methods to attach

If you want to attach functions that would become instance methods of your snapshot class, the old way was to define the function in the feature module and include its name in the list named functions. The new way is to decorate the function with the @function_feature decorator.

(Temporary?) Code to generate new Parameters from old style feature modules

I think the new Parameter approach is more intuitive, but for now I've implemented methods in FeatureCollection that actually convert the old version to the new version. TODO: details to come.

Unchanged: @property

Just as before, and @property in a feature module becomes a property of the snapshot class.