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 support for Boresch modifications to ghost atom bonded terms #55

Merged
merged 60 commits into from
Sep 27, 2024

Conversation

lohedges
Copy link
Contributor

@lohedges lohedges commented Sep 9, 2024

This PR adds support for modifications to ghost atom bonded terms in order to avoid spurious couplings to the physical system, which can lead to physical distortions to the equilibrium geometry in the end states. This is implemented following the logic in this paper with full support for single, dual, triple, and higher-order junctions, including automatic optimisation of equilibrium values for specific angle modifications.

The code has been tested using the example modifications in table 3 of the paper, with unit tests asserting that the bonded-term modifications agree precisely with those in the table. In addition, the code has also been tested using two problem perturbations from the TKK2 data set. The first, ejm42 <--> ejm54 exhibits a perturbation from an sp3 carbon to a sp2 nitrogen. Without modification to the ghost atom bonded terms, the lambda = 1 state exhibits incorrect angle distributions around the perturbed nitrogen. Applying the Boresch modifications fixes this issue:

angles_boresch

Similarly, for jmc30 <--> jmc28 perturbation, which is an sp to sp3 carbon transformation, the lambda = 0 angle distributions around the perturbed carbon are incorrect. Once again, applying the modifications is shown to resolve the issue:

angles_boresch2

The modifications are now applied as default when not using somd1 compatibility mode. If necessary, we can add a keyword (and CLI option) to enable/disable them if preferred. There are also a few configurable parameters for which I've used the defaults recommended in the paper. These could also be exposed if needed, but I think the defaults should be suitable in essentially all cases.

The PR also includes some minor refactoring. Specifically:

  • Standardisation to the use of "ghost" rather than "dummy" throughout the code base, in consistency with the Sire code.
  • Moving of common functionality to an internal somd2._utils utility module.

I aim to test this PR against some existing data sets before merging, e.g. hydration free energies and the full TYK2 set. This will ensure that the modifications don't break existing functionality.

@lohedges lohedges added enhancement New feature or request cresset Related to work with Cresset labels Sep 9, 2024
@lohedges
Copy link
Contributor Author

lohedges commented Sep 9, 2024

Just to say that while this functionality is currently embedded in somd2, there's no reason why it couldn't be moved to a stand-alone function/package. That way it could be used by somd1, too. (Or AMBER and GROMACS, although they have some of their own logic for doing things internally.) The only thing specific to somd2 is that angle/dihedral terms that are removed a deleted from the set of respective three- and four-angle functions, where sire itself re-adds the missing term at the opposite end state for you, i.e. it matches zeros the force constant, but matches the other parameters in the term, e.g. the equilibrium value. For somd1, you need to retain the all functions, then modify them during the writing of the pert file.

@lohedges
Copy link
Contributor Author

lohedges commented Sep 9, 2024

I've also fixed the issue with the macOS CI and have decided to skip the Windows build until we can (locally) resolve the issue with the stale DCD file handle breaking the CI teardown.

@lohedges lohedges requested a review from mb2055 September 9, 2024 15:20
mb2055
mb2055 previously approved these changes Sep 27, 2024
Copy link
Contributor

@mb2055 mb2055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 👍 It will be nice to fully test the problematic systems now that these corrections are implemented

@lohedges lohedges merged commit 666f707 into main Sep 27, 2024
5 checks passed
@lohedges lohedges deleted the feature_ghost_handling branch September 27, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cresset Related to work with Cresset enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants