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

from_openmm doesn't take System masses #1004

Closed
IAlibay opened this issue Jul 12, 2024 · 3 comments · Fixed by #1008
Closed

from_openmm doesn't take System masses #1004

IAlibay opened this issue Jul 12, 2024 · 3 comments · Fixed by #1008
Labels
documentation Improvements or additions to documentation

Comments

@IAlibay
Copy link

IAlibay commented Jul 12, 2024

Description

Reading in an OpenMM System with masses set should be reflected in the generated Interchange object (or maybe I'm looking at the wrong thing - sorry if that's the case!)

Reproduction

from openff.interchange import Interchange
from openff.toolkit import Molecule, ForceField
from openff.units import unit
import numpy as np
import os
os.environ['INTERCHANGE_EXPERIMENTAL'] = "1"

mol = Molecule.from_smiles("CC")
mol.generate_conformers()
sage = ForceField("openff-2.0.0.offxml")
cubic_box = unit.Quantity(30 * np.eye(3), unit.angstrom)

interchange = Interchange.from_smirnoff(topology=[mol], force_field=sage, box=cubic_box)
interchange.positions = mol.conformers[0]
omm_system = interchange.to_openmm(hydrogen_mass=4)
omm_top = interchange.to_openmm_topology()

interchange2 = Interchange.from_openmm(omm_system, omm_top)

mass1 = [at.mass for at in interchange.topology.atoms]
mass2 = [at.mass for at in interchange2.topology.atoms]
mass3 = [omm_system.getParticleMass(i) for i in range(omm_system.getNumParticles())]

print("mass1: ", mass1)
print("mass2: ", mass2)
print("mass3: ", mass3)

Output

mass1:  [<Quantity(12.01078, 'dalton')>, <Quantity(12.01078, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>]
mass2:  [<Quantity(12.01078, 'dalton')>, <Quantity(12.01078, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>, <Quantity(1.007947, 'dalton')>]
mass3:  [Quantity(value=3.0346209999999996, unit=dalton), Quantity(value=3.0346209999999996, unit=dalton), Quantity(value=4.0, unit=dalton), Quantity(value=4.0, unit=dalton), Quantity(value=4.0, unit=dalton), Quantity(value=4.0, unit=dalton), Quantity(value=4.0, unit=dalton), Quantity(value=4.0, unit=dalton)]

Suggested solution

I realised that both:

  1. from_openmm is experimental
  2. HMR within an interchange object probably going to break a ton of stuff

My suggestion would be to do a quick check when loading from openmm. If the masses don't agree then throw a user warning to tell folks that they just lost their non-standard masses.

Software versions

@IAlibay
Copy link
Author

IAlibay commented Jul 12, 2024

Looks like it might be a duplicate of #888?

@mattwthompson
Copy link
Member

Yes, this is (mostly) a duplicate. Unfortunately the best workaround I can suggest, on top of better warning/documenting this here, is just for the user to manage it themselves and remember about the HMR settings at export time.

If you're suggesting that the state of the Interchange should reflect the non-physical masses of the atoms, I agree that's a reasonable expectation but the toolkit doesn't allow masses to diverge from what their atomic number defines1. I haven't thought through how a special representation could work, but it might be doable in the future if it's worth the diverging code paths.

Footnotes

  1. This is a design I advocated for, forgetting about HMR at the time. Usually atomic masses are strictly defined by an atom's element, so it seemed like dead weight at the time.

@IAlibay
Copy link
Author

IAlibay commented Jul 12, 2024

If you're suggesting that the state of the Interchange should reflect the non-physical masses of the atoms

Ah I should have probably clarified: as a user I generally think it's a good call for Interchange to be "physical things only". That sets clear expectations on what you can and can't do.

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

Successfully merging a pull request may close this issue.

2 participants