{"action":"created","issue":{"url":"https://api.github.com/repos/choderalab/openpathsampling/issues/444","repository_url":"https://api.github.com/repos/choderalab/openpathsampling","labels_url":"https://api.github.com/repos/choderalab/openpathsampling/issues/444/labels{/name}","comments_url":"https://api.github.com/repos/choderalab/openpathsampling/issues/444/comments","events_url":"https://api.github.com/repos/choderalab/openpathsampling/issues/444/events","html_url":"https://github.com/choderalab/openpathsampling/issues/444","id":140162651,"number":444,"title":"Revisiting new `Snapshot` structure","user":{"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},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":9,"created_at":"2016-03-11T11:37:56Z","updated_at":"2016-03-14T14:11:15Z","closed_at":null,"body":"This follows up on some stuff from #429, based on a little experience actually using that stuff in practice.\r\n\r\n### `copy` and `deepcopy`\r\n\r\nAs discussed previously, I often want to make a copy of a snapshot where certain features (`coordinates`, `velocities`) are deep copies.  Your solution was to say \"use `copy.deepcopy(snapshot)`.\" However, this is a problem, because some features don't make sense to deepcopy -- e.g., `Topology`. Suppose I'm doing a random walk in `Snapshot` space (two-way shooting moves are an example where we'd do this). I may only change, e.g., the velocity of one atom. So it is very convenient to say \"I want a deep copy of this snapshot, and then I want to change that one part.\"\r\n\r\nA similar thing happens in dynamiq, where I'm sampling initial conditions: in that case, I don't even know *a priori* which features exist.\r\n\r\nWhat I still think I'd like would be a `Snapshot.copy()` function that returns a new snapshot that does a deep copy where relevant and a shallow copy otherwise. Each feature knows whether it is deep or not (topologies/masses aren't, but coordinates/velocities are) so I think that could be as easy as defaulting to deep copies, but letting a feature override with a list like `no_deepcopy = ['topology']` in `topology.py`.\r\n\r\n\r\n### `requires` vs. `provides`?\r\n\r\nThe way things are set up now, we have two separate features for `coordinates` and `xyz`, but `xyz` requires `coordinates` (and you have the `requires` keyword set up to handle that). Could there also be a `provides`, that would allow us to combine `coordinates` and `xyz` into one file, so that any time a you say \"this snapshot should have `coordinates`\" you also get `xyz` for free?\r\n\r\nThis shouldn't replace the `requires` trick; instead it should augment it. For example:\r\n\r\n```python\r\n# coordinates.py\r\nattributes = ['coordinates'] # settable attributes\r\nprovides = ['xyz'] # property\r\n```\r\n\r\nConsider how this makes the following easy:\r\n\r\n```python\r\n# topology.py\r\nattributes = ['topology'] \r\nprovides = ['masses'] \r\n```\r\n```python\r\n# velocities.py\r\nattributes = ['velocities']\r\nrequires = ['masses'] # can come from topology or from elsewhere!\r\nprovides = ['momenta']\r\n```\r\n\r\nI think that would make setup of new snapshot types a lot easier. It would also mean that we could add momenta to our existing snapshots at some future date without breaking backward compatibility. (Although if we implement this, we might as well go the extra step to add momenta now.)\r\n\r\n### One feature with multiple attributes?\r\n\r\nA question: currently, all features follow a few rules:\r\n\r\n* in a file called `feature_name.py`\r\n* contain exactly one attribute; i.e., `attributes = ['feature_name']`\r\n\r\nAre these strict requirements, or is it just that this is the reasonable way to set up these particular features?\r\n\r\nMy interest here is in setting up the monodromy feature for dynamiq. The whole monodromy matrix is a 2Nx2N matrix (for N degrees of freedom). However, it is usually split up into 4 NxN matrices. So I would like for the monodromy feature to look something like:\r\n\r\n```python\r\n# monodromy.py\r\nattributes = ['Mqq', 'Mqp', 'Mpq', 'Mpp']\r\n```\r\n\r\nWill that work?"},"comment":{"url":"https://api.github.com/repos/choderalab/openpathsampling/issues/comments/196324005","html_url":"https://github.com/choderalab/openpathsampling/issues/444#issuecomment-196324005","issue_url":"https://api.github.com/repos/choderalab/openpathsampling/issues/444","id":196324005,"user":{"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},"created_at":"2016-03-14T14:11:15Z","updated_at":"2016-03-14T14:11:15Z","body":"Let me summarize what I think we've agreed:\r\n\r\n* If a user is creating a new feature that doesn't obey the default rules on `copy`, then the user should implement `copy` in `feature.py` (as in approach 3 above). This will automatically be used in snapshots that use this new feature.\r\n* If a user is creating a new snapshot and wants to add functionality to `copy`, there is a decorator which retains the default functionality (or can this be done with just a call to `super().copy`?)\r\n* If a user wants to completely override the functionality of `copy` in a new snapshot, that is possible, but strongly discouraged (as in, no one other than you and me -- make that \"you, and maybe me if I'm looking for a challenge\" -- should try it.)\r\n\r\nIs that accurate?"}}