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

Allow electrostatics in single force + plugins #863

Open
SimonBoothroyd opened this issue Dec 20, 2023 · 4 comments
Open

Allow electrostatics in single force + plugins #863

SimonBoothroyd opened this issue Dec 20, 2023 · 4 comments

Comments

@SimonBoothroyd
Copy link
Contributor

Description

It seems at the moment plugins (e.g. the DEXP potential from smirnoff-plugins) can only be used with combine_nonbonded_forces=False, which leads to the electrostatic 1-4 interactions being split into a custom bond force.

It would be helpful when setting up FE calculations if the electrostatics could be combined in the same force, even if the vdW parameters are not combined.

@mattwthompson
Copy link
Member

Would it be okay if this is implemented at the plugin level? I see why splitting off the 1-4 contribution isn't necessarily a positive value add (the original motivation is for comparing things, and for parity with how GROMACS handles 1-4 interactions). Prior to actually giving it a spin, I think the modify_openmm_forces backdoor would be a quicker path to this change than exposing the toggle as another option everywhere.

@j-wags
Copy link
Member

j-wags commented Jan 5, 2024

@SimonBoothroyd @jthorton - Talking to @mattwthompson, this one needs your feedback to proceed.

@SimonBoothroyd
Copy link
Contributor Author

Thanks @mattwthompson and @j-wags !

My slight concern about moving this to the plugins is that then the plugins then need to start making assumptions about how the 1-4 interactions were initially split out (e.g. need to do the string match "138.*qq/r") which may cause them silently to become incompatible with the upstream interchange implementation.

It would also mean the meaning / ramifications of the combine_nonbonded_forces flag gets a bit messier which might be undesirable!

@mattwthompson
Copy link
Member

I forgot, in the past while facing a similar issue, I resolved to name forces:

In [1]: from openff.toolkit import *

In [2]: x = ForceField("openff_unconstrained-2.0.0.offxml").create_interchange(Molecule.from_smiles("O").to_topology()).to_open
   ...: mm(combine_nonbonded_forces=False)


In [3]: x.getForce(3).getName()
Out[3]: 'Electrostatics 1-4 force'

Do you think relying on this would be an acceptable solution? The others, which are probably improvements but also sweeping behavior changes, would be more or less

  1. Introduce another argument merge_electrostastics_14 that exposes this option to the user, possibly wired through to the toolkit as well
  2. Simply combine the 1-4 interactions by default and, for testing purposes, make the above argument private

therefore, in order to get your free energy calculations working more quickly, I'm inclined to make this change in plugins and push default changes into the future

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

No branches or pull requests

3 participants