Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a method to copy a Universe #1249

Closed
jbarnoud opened this issue Mar 19, 2017 · 6 comments
Closed

Add a method to copy a Universe #1249

jbarnoud opened this issue Mar 19, 2017 · 6 comments

Comments

@jbarnoud
Copy link
Contributor

This issue follows a comment by @kain88-de: #1027 (comment)

Copying a Universe, while is looks easy, can go wrong in multiple ways. When copying a Universe, one must think about, at least, these things:

  • was the topology built from a file or not? If so, a new topology can maybe be built from the original file.
  • was the topology modified since it got read? If so, building from file will be wrong.
  • is the trajectory reader reading a file?
  • is the trajectory a MemoryReader? If so, was it created from a file? Did the coordinates change since then?
  • has the trajectory additional data attached to it?
  • Is there any auxliary attached?
  • ...

A copy method in Universe would streamline the copy.

One way to implement such method would be to have a copy method in readers, topologies, and auxiliaries. The standard case would be implemented in the base class. Specific case, like the memory reader, would overwrite the method.

Being able to copy AtomGroups and to attach them to a copy of a universe could also come handy, but is out of scope of the proposal.

@richardjgowers
Copy link
Member

A lazy first effort (and probably valid for most cases) would be to read filenames and just recreate a Universe off those files, but a much better and rigorous approach is to properly copy the potentially modified objects.

Another thing to bear in mind is whether we're doing deep copies or shallow copies of things. This will be most notable when dealing with numpy arrays, and are we copying a view of the data (ie making a copy of a pointer for ourselves) or making an independent copy of the array. Probably better to make sure we're making copies of the numpy arrays.

Definitely worth making Universe.copy just operate on Topology.copy and Trajectory.copy. If we solve what copying a Topology means in terms of data, we've also half (or more) solved #643

Similarly, if we can copy a trajectory, we are much closer to serialising, so that's also a huge step towards our parallel efforts.

@kain88-de
Copy link
Member

I guess we might want to have a copy and deepcopy method where the first is a shallow copy method.

@jbarnoud
Copy link
Contributor Author

As a note: there is a prototype for #643 in the closed pull request #831

@orbeckst
Copy link
Member

I very much like the idea of adding copy (or deepcopy) to the topology and trajectory/auxiliary objects. That makes the task somehow look more manageable.

We could start with a highly experimental version, which just does what probably most people have been doing when they cloned a universe: just re-read from files. Add NotImplementedErrors for cases where we know that this is going to fail, and clean up while we go along.

@jbarnoud
Copy link
Contributor Author

Duplicate of #1029

@jbarnoud jbarnoud marked this as a duplicate of #1029 Jul 15, 2017
@orbeckst
Copy link
Member

I didn't know that GitHub knows how to mark issues as duplicates!

Are there other magic words for issue comments ... ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants