Conversation
….e. if improper_style is cvff
…t file are left out
|
One small comment but if this fixes your examples then this LGTM |
| convertfunc = convert_dihedral_from_fourier_to_trig | ||
| converted_dihedral = TrigDihedral | ||
| elif dihedral == TrigDihedral: | ||
| convertfunc = lambda x: x |
There was a problem hiding this comment.
Could you either remove the converted_dihedral = dihedral # Default line above or move the convertfunc to the top as the base case and delete this branch?
|
@ctk3b, if you had a chance to review some of these and accept, since you wrote chunks of it and are now a better coder that me, it might be useful :). No worries if you can't . . . |
|
Yup! I don't have time to get LAMMPS up and running locally and confirm this but the change looks correct to me. If @orioncohen can confirm this works for some LAMMPS files then I would recommend merging this |
|
@orioncohen - how about you write the proper tests to verify the new functionality works, and then we get it checked in? |
|
I can take a look at this (I have LAMMPS installed, etc.) if provided a few input files. I can also make a release once this is merged in, if that's more convenient than installing from source. |
|
Thanks for the feedback everyone. I've had a chance to play with this a bit more, and while this has allowed me to read and write my files, but they are being corrupted in the process. The impropers are being removed and the dihedral_style is being incorrectly identified as charmm despite specifying opls in the input. Whatever the cause, clearly my "fix" is allowing something to happen that should not. I don't have the debugging time to get to the heart of this. @mattwthompson, if you want to take a look at this I'd be happy to send along some input files. |
|
Could you provide some input files so this can be tested out? |
|
Sorry for the delay. I just committed some sample input files. |
|
Thanks - I'll have a look but it might take me about week to get to it 🙂 |
|
I can't get that script + data file to run out of the box - does it work on your machine? I'm lazy and using somebody else's LAMMPS build, but I'm pretty sure it has the necessary packages built with it: https://github.com/conda-forge/lammps-feedstock/blob/4098a282c2ade64a27f243f4a7926acc9dc2b90f/recipe/build.sh#L3 |
|
The file I sent won't actually run, it is a subset of a valid file that only contains the information that InterMol parses. Sorry that I wasn't clear. I left in only the parts that are relevant to the IO problem at hand, I can send a more complete file if that would be helpful though. I can't even get that extremely minimal file to be loaded by Intermol. (and I am 99% sure that I included all of the keywords that InterMol searches for). |
|
Dear developers, |
This is meant to address Issue #378, which identifies bugs with the dihedral_style
oplsand the improper_stylecvff.It uses some code from PR #349, written by @JoshuaSBrown. I rewrote the code here because that PR had not been touched in 2 years.