{"action":"created","issue":{"url":"https://api.github.com/repos/choderalab/openpathsampling/issues/434","repository_url":"https://api.github.com/repos/choderalab/openpathsampling","labels_url":"https://api.github.com/repos/choderalab/openpathsampling/issues/434/labels{/name}","comments_url":"https://api.github.com/repos/choderalab/openpathsampling/issues/434/comments","events_url":"https://api.github.com/repos/choderalab/openpathsampling/issues/434/events","html_url":"https://github.com/choderalab/openpathsampling/pull/434","id":134561566,"number":434,"title":"Restructuring of Engines and Dynamics","user":{"login":"jhprinz","id":5091934,"avatar_url":"https://avatars.githubusercontent.com/u/5091934?v=3","gravatar_id":"","url":"https://api.github.com/users/jhprinz","html_url":"https://github.com/jhprinz","followers_url":"https://api.github.com/users/jhprinz/followers","following_url":"https://api.github.com/users/jhprinz/following{/other_user}","gists_url":"https://api.github.com/users/jhprinz/gists{/gist_id}","starred_url":"https://api.github.com/users/jhprinz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jhprinz/subscriptions","organizations_url":"https://api.github.com/users/jhprinz/orgs","repos_url":"https://api.github.com/users/jhprinz/repos","events_url":"https://api.github.com/users/jhprinz/events{/privacy}","received_events_url":"https://api.github.com/users/jhprinz/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"dwhswenson","id":8178546,"avatar_url":"https://avatars.githubusercontent.com/u/8178546?v=3","gravatar_id":"","url":"https://api.github.com/users/dwhswenson","html_url":"https://github.com/dwhswenson","followers_url":"https://api.github.com/users/dwhswenson/followers","following_url":"https://api.github.com/users/dwhswenson/following{/other_user}","gists_url":"https://api.github.com/users/dwhswenson/gists{/gist_id}","starred_url":"https://api.github.com/users/dwhswenson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dwhswenson/subscriptions","organizations_url":"https://api.github.com/users/dwhswenson/orgs","repos_url":"https://api.github.com/users/dwhswenson/repos","events_url":"https://api.github.com/users/dwhswenson/events{/privacy}","received_events_url":"https://api.github.com/users/dwhswenson/received_events","type":"User","site_admin":false},"milestone":null,"comments":26,"created_at":"2016-02-18T12:03:37Z","updated_at":"2016-03-06T08:17:10Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/choderalab/openpathsampling/pulls/434","html_url":"https://github.com/choderalab/openpathsampling/pull/434","diff_url":"https://github.com/choderalab/openpathsampling/pull/434.diff","patch_url":"https://github.com/choderalab/openpathsampling/pull/434.patch"},"body":"This will rearrange all the parts involved in the generation of trajectories and snapshots into a separate module of OPS. I so far picked the name `openpathsampling.dynamics`\r\n\r\n### Things to finish\r\n\r\n- [x] restructuring of folders\r\n- [x] fix imports\r\n- [x] fix tests\r\n- [x] fix notebooks\r\n- [x] add error message if no dynamics engine has been imported and you are trying to load from storage.\r\n- [x] refactor docs (at least added engines to a minimum)\r\n\r\n### Questions?\r\n\r\n1. How to name the package? I chose `dynamics` but maybe there is a better one\r\n2. How to name the standerd import name in `import openpathsampling.dynamics as ???`. So far I used `dyn` and kept `toys` for the ToyDynamics Example\r\n3. Right now loading and saving only works, if the appropriate engine has been imported. What does that mean? If you load then just by running `import openpathsampling as paths` you will no include ANY engine (which makes sense) and hence opening a storage that used a particular engine will fail. Since storage has no concept of creating engines of a particular type and the associated snapshots. This means without the line `import openpathsampling.dynamics.toy` you cannot run `storage = Storage(''mstis.nc\")`. But the simple import is enough to fix this. For creating a store it is mostly no problem since you need a Snapshot of the correct type to create the storage in the first place.\r\n4. How to deal with shared objects? Right now I have put all shared files into the dynamics folder and use the `__init__.py` there to include all the shared objects. This means that `openpathsampling.dynamics` also looks almost like an engine module itself. It just has no implementation. All the other engine module then _derive_ from dynamics. I think this is nice and the package structure reflects nicely these dependence. It is almost like subclassing modules.\r\n\r\n### Outline\r\n\r\nThe current structure is as follows\r\n```\r\nengines\r\n├── __init__.py\r\n├── dynamics_engine.py\r\n├── snapshot.py\r\n├── topology.py\r\n├── trajectory.py\r\n├── features\r\n│   ├── __init__.py\r\n│   ├── base.py\r\n│   ├── shared.py\r\n│   ├── box_vectors.py\r\n│   ├── coordinates.py\r\n│   ├── ...\r\n│   └── xyz.py\r\n├── openmm\r\n│   ├── __init__.py\r\n│   ├── features\r\n│   │   └── __init__.py\r\n│   ├── engine.py\r\n│   ├── snapshot.py\r\n│   ├── tools.py\r\n│   └── topology.py\r\n├── toy\r\n│   ├── __init__.py\r\n│   ├── engine.py\r\n│   ├── features\r\n│   │   └── __init__.py\r\n│   ├── integrators.py\r\n│   ├── pes.py\r\n│   ├── snapshot.py\r\n│   └── topology.py\r\n└── ...\r\n```\r\n\r\nSo the structure of an engine module is\r\n```\r\n└── engine\r\n    ├── __init__.py        # make all part available\r\n    ├── engine.py          # logic of the engine\r\n    ├── features           # additional features\r\n    │   ├── __init__.py    # import features from shared ones and add new features\r\n    │   ├── feature1.py    # implementation of new features\r\n    │   └── ...  \r\n    ├── snapshot.py        # implementations of new snapshots\r\n    ├── tools.py           # implementations utility functions\r\n    └── topology.py        # implementation of new topologies \r\n```\r\n\r\nIf you want to use a specific engine then import it using\r\n```\r\nimport openpathsampling.dynamics.engine as eng\r\n\r\nengine = eng.Engine(...)\r\n```\r\n\r\nthe top_level `__init__.py` will take care that the module will reveal all necessary function and classes when imported. There will be two classes (usually just references) that should (must?) be present in each engine:\r\n\r\n1. `Engine()` : This is the default engine to be used\r\n2. `Snapshot()` : The default used snapshot so that the following lines work independently for each engine\r\n\r\n```\r\nengine = eng.Engine()\r\nsnapshot = eng.Snapshot()\r\n```\r\n\r\nalthough the creation is of course engine/snapshot dependent. An example for the `__init__.py` from openmm looks like\r\n\r\n```py\r\nfrom engine import OpenMMEngine as Engine\r\nfrom tools import (\r\n    empty_snapshot_from_openmm_topology,\r\n    snapshot_from_pdb,\r\n    snapshot_from_testsystem,\r\n    to_openmm_topology,\r\n    trajectory_from_mdtraj\r\n)\r\n\r\nimport features\r\n\r\nfrom snapshot import Snapshot, MDSnapshot\r\n```\r\n\r\nthe features will always be imported to allow for\r\n```\r\nfeat1 = eng.features.[feature1]\r\n```\r\n\r\nThe features `__init__.py` for openmm looks like\r\n```py\r\nfrom openpathsampling.dynamics.features import *\r\nfrom openpathsampling.dynamics.features.shared import Configuration, Momentum\r\n```\r\n\r\nnote the dangerous `*` which is explicitely wanted here since OpenMM should have access to all the features. For ToyDynamics we have limited these\r\n\r\n```py\r\nfrom openpathsampling.dynamics.features import (\r\n    base,\r\n    coordinates,\r\n    velocities,\r\n    topology,\r\n    xyz\r\n)\r\n```"},"comment":{"url":"https://api.github.com/repos/choderalab/openpathsampling/issues/comments/192827712","html_url":"https://github.com/choderalab/openpathsampling/pull/434#issuecomment-192827712","issue_url":"https://api.github.com/repos/choderalab/openpathsampling/issues/434","id":192827712,"user":{"login":"jhprinz","id":5091934,"avatar_url":"https://avatars.githubusercontent.com/u/5091934?v=3","gravatar_id":"","url":"https://api.github.com/users/jhprinz","html_url":"https://github.com/jhprinz","followers_url":"https://api.github.com/users/jhprinz/followers","following_url":"https://api.github.com/users/jhprinz/following{/other_user}","gists_url":"https://api.github.com/users/jhprinz/gists{/gist_id}","starred_url":"https://api.github.com/users/jhprinz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jhprinz/subscriptions","organizations_url":"https://api.github.com/users/jhprinz/orgs","repos_url":"https://api.github.com/users/jhprinz/repos","events_url":"https://api.github.com/users/jhprinz/events{/privacy}","received_events_url":"https://api.github.com/users/jhprinz/received_events","type":"User","site_admin":false},"created_at":"2016-03-06T08:17:10Z","updated_at":"2016-03-06T08:17:10Z","body":"Removed some redundant code in `.engines`. There is now only one `Snapshot` implementation for each type. As an example the ToySnapshot is defined using the SnapshoFactory while the openmm ones are still.\r\n\r\nOne note on inheritance for features.\r\n\r\nIf you subclass, then all features are just joined: Assume the base class has features `coor` and `velo` and your new only implements `topology` then the new snapshot will have all of them with all their properties, etc. This can lead to clashed which will be caught as a RuntimeError.\r\n\r\nIf you want to use it, let me know if this is actually what you need. \r\n\r\nThere is actually one (minor) limitation to the use of the Factory over the decorator. If you want to add functions like the `.construct` in the `openmm.Snapshot` it is very tricky to add this when you use the factory. In this case I only have the example use the decorator."}}