shirtsgroup / InterMol

Conversion tool for molecular simulations
MIT License
185 stars 53 forks source link

Several bugs in Lammps IO #378

Closed orionarcher closed 2 years ago

orionarcher commented 2 years ago

I've encountered several bugs in the LAMMPS IO when using 'opls' dihedral type and 'cvff' improper style. I believe that I can fix them, but I am wondering if those fixs would be merged.

One of the errors is already addressed by PR #349, but that has been open for two years.

The other error occurs in the canonical_dihedral function in lammps_parser.py.

    def canonical_dihedral(self, params, dihedral, direction='into'):
        """Convert from the canonical form of this interaction. """

        if direction == 'into':
            converted_dihedral = dihedral  # Default
            if dihedral == ProperPeriodicDihedral:  # Proper dihedral
                convertfunc = convert_dihedral_from_proper_to_trig
                converted_dihedral = TrigDihedral
            elif dihedral == ImproperHarmonicDihedral:
                convertfunc = convert_nothing
            elif dihedral == RbDihedral:
                convertfunc = convert_dihedral_from_RB_to_trig
                converted_dihedral = TrigDihedral
            elif dihedral == FourierDihedral:
                convertfunc = convert_dihedral_from_fourier_to_trig
                converted_dihedral = TrigDihedral
                # Now actually convert the dihedral.
            params = convertfunc(params)

If dihedral == TrigDihedral at the beginning, then convertfunc will never be defined and an error will occur. An easy fix.

If I create a PR and fix these issues, is any developer available to review and merge them? I'd like to contribute but it appears the code has not been updated in some time.

mrshirts commented 2 years ago

Hi, Orion-

I have run out of time to support InterMol right now because of lack of funding/students and because of the future development of interchange. See https://github.com/ParmEd/ParmEd/issues/1204 for more discussion . . . I'd like to promise review, but because we never set up great testing on InterMol when we started in the olden days, I can't guarantee it won't break things, and I currently don't have time to do that sort of testing when I have grad students to mentor on their projects.

yel21 commented 2 years ago

Dear Sirs,

If any chance to solve Intermol problems that related to dihedral angles for conversion from LAMMPS to GROMACS?

orionarcher commented 2 years ago

Hello @yel21,

Unfortunately this issue was never resolved. As alluded to above above, this is a planned feature for the not-yet-completed force field interchange library. Until that package is more mature, it is unlikely we'll have a solution to cross-application force field conversion.

In the meantime, the OpenFF Toolkit has some great tools for parameterizing forcefields.

yel21 commented 2 years ago

Hello @yel21,

Unfortunately this issue was never resolved. As alluded to above above, this is a planned feature for the not-yet-completed force field interchange library. Until that package is more mature, it is unlikely we'll have a solution to cross-application force field conversion.

In the meantime, the OpenFF Toolkit has some great tools for parameterizing forcefields.

Hello @orioncohen, Thank you for answer.