Detect and remove duplicate dihedrals with the same periodicity #530
Conversation
LSchwiebert
requested review from
jpotoff,
GregorySchwing,
bc118,
go2213 and
HamiedAsadi
|
Updated patch to generate an error and exit, instead of a warning, if the duplicate dihedrals have different parameter values. |
src/FFSetup.cpp
Outdated
| } | ||
|
|
||
| if (duplicate) { | ||
| std::cout << "Warning: Skipping duplicate periodicity of " << index |
There was a problem hiding this comment.
This patch works as intended per my testing.
I looked at what NAMD does. It appears they take the last one input. We may want to be consistent with them for py-mcmd.
My last comment, is more of a question, and this would go for non-bonded and all bonded parameters.
Do we want to more than NAMD and check if the parameters are the same or different (this would be more than NAMD):
- if the same, skip it
- if different, throw an error or overwrite with the last one.
There was a problem hiding this comment.
I tested is and it Looks like it is working as intended.
There was a problem hiding this comment.
I updated the patch so that it throws an error if the parameters differ by more than 0.001. I have to imagine that would be a mistake. Since it only proceeds if they match, then which one we save doesn't matter.
So, we do the same thing as NAMD if the parameters match except we also issue a warning, and we terminate with an error if the parameters differ.
|
Reviewed and it looks good. My only suggested change is some wording. Change the below "Using first entry." to "found duplicate entry" or similar, as it errors out if the entries are different, other than the n-value, which the n-value has to be allowed to be different in this periodic form because it is listed as n entries. Reading DIHEDRALS parameters. |
|
Thanks for the feedback. I have revised the error messages per your suggestions. |
Resolves issue #529 by issuing a warning when dihedrals with matching periodicities are found. The duplicates are removed and a warning is generated. The warning looks like this:
Warning: Skipping duplicate periodicity of 3 for dihedral HCCTCTHC!