openmm / openmm-ml

High level API for using machine learning models in OpenMM simulations
Other
75 stars 25 forks source link

Question about removing non bonded forces for ML potential substituion #55

Closed JSLJ23 closed 10 months ago

JSLJ23 commented 10 months ago

I was trying to adapt the code to allow for more specific customisation of hybrid systems which allow ML potential for the protein and ligand, while using MM potentials for the solvent due to the VRAM limits on GPUs.

Within the createMixedSystem() function of MLPotential, there is this block of code which I don't quite understand why it is written in this way:

  atomList = list(atoms)
  for force in newSystem.getForces():
      if isinstance(force, openmm.NonbondedForce):
          for i in range(len(atomList)):
              for j in range(i):
                  force.addException(i, j, 0, 1, 0, True)
  1. If atoms is not a continuous list of atom indexes (i.e. [1, 3, 5, 6, 7, 8]), won't the i in range(len(atomList)) fail to properly obtain the atom indexes for adding Exceptions?
  2. Shouldn't the adding of exceptions be force.addException(atomList[i], atomList[j], 0, 1, 0, True)
  3. Does the force.addException() function work separately with respect to the first and second particle? For instance can particle A experience the force contribution by particle B but not the other way around?
peastman commented 10 months ago

Shouldn't the adding of exceptions be force.addException(atomList[i], atomList[j], 0, 1, 0, True)

Yes, I believe you're correct about that. Thanks for catching it!

For instance can particle A experience the force contribution by particle B but not the other way around?

No, it has to be symmetric. The interaction between A and B only gets computed once and then applied to both particles. It doesn't matter which order you list them in.

peastman commented 10 months ago

The fix is in #56. Can you try it and confirm whether it works for you?

JSLJ23 commented 10 months ago

Thank you for getting back so quickly! Yes the fix works as intended now! 👍

peastman commented 10 months ago

Thanks!