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

Combine force fields in-memory #1948

Open
mattwthompson opened this issue Oct 21, 2024 · 1 comment
Open

Combine force fields in-memory #1948

mattwthompson opened this issue Oct 21, 2024 · 1 comment

Comments

@mattwthompson
Copy link
Member

Is your feature request related to a problem? Please describe.

I frequently wish to combine force fields, like Sage with a different water model. This works fine when loading from disk:

sage_with_opc = ForceField("openff-2.2.1.offxml", "opc.offxml")

However, if I'm in the middle of a workflow and have already loaded them up into memory, there's no straightforward way to combine ForceFields.

Describe the solution you'd like

It'd be nice to be able to do something like

sage = SmallMoleculeForceFieldProvider.get("openff")
opc = WaterModelProvider.get("opc")
sage_with_opc = sage.combine(opc)

Parameter ordering, parameter section compatibility, etc. should follow the existing behavior.

Describe alternatives you've considered

I can handle this with an extra trip to disk, but this is slow and I'd argue should not be necessary.

sage.to_file("file1.offxml")
opc.to_file("file2.offxml")
sage_with_opc = ForceField("file1.offxml", "file2.offxml")

Additional context

Combining force fields has behavior consequences which are potentially significant and often result in worse performance. Combining force fields, however, is part of the specification and currently supported in de-serialization, so I don't think it's reasonable to claim this behavior out to be unsupported.

@j-wags
Copy link
Member

j-wags commented Oct 22, 2024

Agree that this would be great, and with your specific proposal to add a combine method. I'd be happy to take a PR for this, and we could probably refactor the ForceField initializer to use this code path when multiple sources are given as input as well.

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

No branches or pull requests

2 participants