Update topology/system particle bookkeeping in OpenMM #1051
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -173,6 +186,21 @@ def to_openmm_topology( | |||
| order=bond_order, | |||
| ) | |||
|
|
|||
| if has_virtual_sites and not collate: | |||
| virtual_site_chain = openmm_topology.addChain(atom_chain_id) | |||
| virtual_site_residue = openmm_topology.addResidue("VS", virtual_site_chain) | |||
There was a problem hiding this comment.
From a practical standpoint, a reference to the original residue would possibly be useful - i.e. when I'm doing "solvent within N of Y" I'm looking for a residue name that matches SOL | WAT | HOH not VS.
| virtual_site_residue = openmm_topology.addResidue("VS", virtual_site_chain) | ||
|
|
||
| for molecule_index, molecule in enumerate(topology.molecules): | ||
| virtual_sites_in_this_molecule: list[VirtualSiteKey] = molecule_virtual_site_map[molecule_index] |
There was a problem hiding this comment.
Only briefly looking at things, but it looks to me like that we're really needing is this map. If we could map things to/from the collated form then yes we have to plan for this map being optionally around, but at the same time it's not crazy extra amounts of work to work around it.
| interchange | ||
| The Interchange object to convert to an OpenMM Topology. | ||
| collate | ||
| If False, the default, all virtual sites will be added to a single residue at the end of the topology. |
There was a problem hiding this comment.
single residue
I'm sure I'm missing something important here - is there a practical advantage to doing a single residue rather than a residue per molecule's worth of virtual sites?
Description
Resolves #1049
Checklist