-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix issues 213, 215, and 219 #220
Conversation
This looks good - I am happy with the code you've used to detect molecules. It is very simple. We can look to optimise it in the future if needed. I guess this would only possibly impact things when we have, e.g. perturbable proteins? |
Yes, I doubt it's much of a performance issue. If needed, it's trivial to keep track of the molecule indices as the ghosts are detected. I just did this as the simple first fix and thought I'd optimise once we knew it fixed the problem. (And if optimisation was needed.) |
This PR closes #219 by only excluding non-bonded interactions between from/to ghost atoms within the same molecule. This is one of several possible solutions (probably) the easiest. The issue in #219 can also be avoided by using a distance restraint between the alchemical ion and perturbable molecule, but this isn't used in
somd1
and the implementation works without issues. Regardless, I think that the logic to exclude the non-bonded interactions was only intended for atoms within a perturbable molecule anyway.In the fix I've just reused the existing
start_indices
vector to work out the molecule index. I'm happy to also create aatom_to_mol
container or similar to avoid the double molecule index search. (This was just a quick fix, re-using what we had.)Note that I've currently skipped the CI. I can trigger once we resolved issue #217. This PR supersedes #216.
devel
into this branch before issuing this pull request (e.g. by runninggit pull origin devel
): [y]Suggested reviewers:
@chryswoods