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

Error with addExclusion() #38

Open
JohannesKarwou opened this issue Sep 5, 2022 · 4 comments
Open

Error with addExclusion() #38

JohannesKarwou opened this issue Sep 5, 2022 · 4 comments

Comments

@JohannesKarwou
Copy link

For certain molecules, like the phenylformate example here:
phenylformate.zip
I get an error when creating a mixedSystem:
ml_system = potential.createMixedSystem(psf.topology, mm_system, ml_atoms, interpolate=True)

TypeError: addExclusion() takes 3 positional arguments but 4 were given

When I look in the code of openmmm-ml:

force.addExclusion(i, j, True)

I see that the particles i and j are passed to the function together with a boolean. Is this really necessary, because the corresponding addExclusion function is only expecting the two particles? (I'm using openmm version 7.7.0)

    def addExclusion(self, particle1, particle2):
        r"""
        addExclusion(self, particle1, particle2) -> int
        Add a particle pair to the list of interactions that should be excluded.

        In many cases, you can use createExclusionsFromBonds() rather than adding each exclusion explicitly.

        Parameters
        ----------
        particle1 : int
            the index of the first particle in the pair
        particle2 : int
            the index of the second particle in the pair

        Returns
        -------
        int
            the index of the exclusion that was added
        """
        return _openmm.CustomNonbondedForce_addExclusion(self, particle1, particle2)

When removing True everything seems to work fine...

@peastman
Copy link
Member

peastman commented Sep 5, 2022

Can you post your script as well? Without knowing the values of mm_system and ml_atoms, I can't tell what's going on or what might be causing the problem.

@JohannesKarwou
Copy link
Author

Thanks for the fast response!

I created a minimal example (minimal_example.py), so one can reproduce the error:
minimal_example.zip

I noticed, that I can reproduce the error only for systems containing ions:

  • For reproducing the error, the CHARMM files phenylformate_with_ions.psf and phenylformate_with_ions.crd are read in. When printing the mm_system I see an additional openmm.CustomNonbondedForce, which seems to cause the error message mentioned above.
  • If I remove the Ions from the system (phenylformate.psf and phenylformate.crd) everything works fine (I do not have the openmm.CustomNonbondedForce and thus run not into the addExclusion() function).

If you need any additional information, let me know!

@peastman
Copy link
Member

peastman commented Sep 7, 2022

createMixedSystem() with interpolate=True doesn't work with CustomNonbondedForces. Possibly we could figure out a way to make it work, but that could be challenging. Otherwise, we should probably make it throw an exception in that case stating exactly what the problem is.

@jchodera
Copy link
Member

jchodera commented Sep 7, 2022

@dominicrufa may have input as well.

Would simply moving both the real Forces and the ML Force into the CustomCVForce and interpolating between the two that way provide a workable solution?

EDIT: Nevermind this proposal. I was thinking we had explored an alternative strategy, but I don't think this would help in this case.

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