shirtsgroup / InterMol

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

sum_parameters? #336

Open mrshirts opened 7 years ago

mrshirts commented 7 years ago

I'm noticing code in the desmond_parser.py

    # this will fail if it's the wrong type of dihedral                                                       
                if dihedralmatch:
                    dihedralmatch.sum_parameters(dihedral)

However, sum_parameters doesn't seem to be defined anywhere. Christoph, do you remember what might have happened in the logic here? I appear to have lost track.

Incidentally, the code appears to work other than throwing a exception that the function doesn't exit; the energies match in the tests I've seen so far. There's just a lot of errors in the logfile that would be good to get rid of.

ctk3b commented 7 years ago

I honestly don't know what this is from. I just grep'd through the full git history and whatever sum_parameters did, it was never checked into the git repo.

The way it's written makes me think it was never properly implemented since we wouldn't be somehow modifying dihedralmatch, we would extract some information from it or somehow add it to a list of parameters to sum.

mrshirts commented 7 years ago

Hmm. OK. I'll dig into this a bit more . . .

xianqiangsun commented 6 years ago

I also experience this problem on the application of sum_parameters, it is not defined anywhere else. The error raised when I was parsing the structures with POPC bilayer.

jro1234 commented 6 years ago

Maybe it is related to the general issue of ff compatibliity? I am converting systems parameterized with charmm22star made with the desmond viparr tool, and have this try-except block produce errors when converting away from the cms file. I have seen that others have not adapted this forcefield accurately even after revisions, so I went with using Maestro and viparr tool (which also is not accurate to the published description due to carrying asp dihedrals over to the asparagines via the shared atom-type strings).

But all that aside, charmm22star has multiple dihedral terms for the same periodicity, so maybe sum_parameters was a method to perform the calculations to reduce these terms? It seems this way of defining dihedrals is generally not compatible with standards in other forcefields and MD programs.