Doesn't TopologyProposal need old_topology as well?

[jchodera]

Otherwise how will we figure out the old atoms that don't appear in the new molecule?

[pgrinaway]

If we have new_to_old_atom_map, we can do something like:

n_old_atoms = old_system.getNumParticles()
unique_old_atoms = [atom for atom in range(n_old_atoms) if atom not in new_to_old_atom_map.values()]

[jchodera]

Hm. OK. I think it would be useful to have convenient access to all the bits of information through TopologyProposal, though:

• new_to_old_atom_map
• old_to_new_atom_map
• old_atoms_to_be_deleted
• old_core_atoms
• new_atoms_to_be_added
• new_core_atoms

etc. Maybe we can make TopologyProposal a class that has methods/fields/properties to retrieve all of these, but they are computed as needed for convenience?

Would still maybe be useful to have the old topology in case we need to write PDB files of these transformations for debugging.

[pgrinaway]

I do generally agree that we should promote TopologyProposal to a class, and that it should have some of those convenience attributes.

[jchodera]

There is a test that is failing because old_topology is needed, I think. Not sure if critical or not.

[pgrinaway]

Ok... looking into this. I thought I had removed that dependency previously.

[pgrinaway]

Ok, these are the import problems I was talking about. I don't think the TopologyProposal has been imported properly in the tests.
(https://travis-ci.org/choderalab/perses#L847)[this] looks like that problem.

There are also some API issues. I'm going to open a PR and promote the TopologyProposal to a class now.

[jchodera]

Ah. topology_proposal is both used as a module name (from perses.rjmc import topology_proposal) and an object name.

Let me fix that.

[pgrinaway]

Oh! Sorry about that!

[pgrinaway]

Addressed by #27