-
Notifications
You must be signed in to change notification settings - Fork 9
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
Possible mis-ordering in carbonyl bond parameters #62
Comments
I had a memory of us making changes to this section before, but I think it was pull request #11 which addressed the difference is |
This also raises a larger question of how we test a change like this. Any thoughts? It seems to me we might ultimately be interested in two levels of tests: Thoughts? |
I agree we need to also discuss that part, I was going to write a separate issue for how to validate changes. |
Here's some details as a part of the carboxylate groups: Here is the section that is relevant part of the
This looks like it is in the correct order, as Stan noted Here are the corresponding lines in
@davidlmobley and @cbayly13 could you confirm that is this the order we want things in? I'll run a couple of example molecules, but I think this is what it should be. |
I'm slightly confused. So you're saying that the order is already correct? So Stan may have been looking at an outdated file with incorrect order? |
I haven't looked too far back, but yes, I think the order is already correct (and in the order Stan suggested it should be). |
Since I'm looking at this section, I think |
I'm trying to find when this was changed: Order was incorrect when this repository was made (first commit) It was updated in PR #43 where there were a lot of changes You can see our full discussion on this same topic in issue #38! |
Haha, excellent, so we fixed it before Stan caught it -- basically just an outdated version of the file -- which means all the validation work was done with the correct FF. :) |
yep, I would like to think about the other parameter I brought up:
It is definitely a corner case, but I think our general rule is to have the earlier SMIRKS patterns as general as they can be. |
I'm going to close this since the ordering was fixed and put my smaller protonated carbonyl question in its own issue. |
Here is an e-mail correspondence considering how parameters are assigned to C=O bonds in protonated carboxylic acids compared to carboxylate groups.
David and I agreed that this seemed reasonable. Note, this sparked two discussions one for the parameters for carboxylate groups and how we evaluate changes to a SMIRNOFF file. This issue will address the former and the latter will be addressed in its own issue (potentially in the openforcefield repository as that is a test we will want for any SMIRNOFF formatted force field now or in the future).
The text was updated successfully, but these errors were encountered: