WBO testing

[sukanyasasmal]

The purpose of this PR is to

• Bring the OE ToolkitWrapper WBO functionality to a working state
• Design and implement sensible validation tests against predictable molecules with known confounding factors
• (maybe this PR, maybe another) Add support for bond order interpolation to the BondHandler class
• (maybe this PR, maybe another) Update the SMIRNOFF spec page to show this functionality is available and state any limitations/changes from preliminary spec
• (maybe this PR, maybe another) Add OFFXML files based on a existing bond-order-interpolating FF
• Update changelog

Status

• Ready for review
• Ready for merge

[lgtm-com]

This pull request introduces 1 alert and fixes 1 when merging a096787 into 4aafa27 - view on LGTM.com

new alerts:

• 1 for Syntax error

fixed alerts:

• 1 for Suspicious unused loop iteration variable

[davidlmobley]

You can also put "[WIP]" in the title to make more clear this is something you're working on.

[codecov-io]

Codecov Report

Merging #430 into master will not change coverage.
The diff coverage is n/a.

[jchodera]

Could you add a bit more information about what this PR intends to accomplish?

[j-wags]

@jchodera -- We've been discussing this in-person, but I should have posted more info in the PR description. I've modified the PR text above with the goals/progress regarding this work.

[j-wags]

@sukanyasasmal I've realized that we haven't specified the extent of this PR (should this cover everything through actual bond parameter assignment?). I think, for now, we just should focus on adding this as a test and ensuring that we reproduce the physical phenomenon we expect (higher bond order in the anion). Once the test is integrated, we can decide if we want this PR to go all the way through parameter assignment.

I've left a few small suggestions as comments. But overall I think this is ready to move into test_toolkits!

[j-wags]

@sukanyasasmal I've made two quick commits on this branch[1], so be sure to run git pull before you make further changes!

I think the new code in is good shape. Thanks again for adding the detailed test -- I have much more faith in our WBO implementation now, and having this test built will allow us to validate the AmberTools implementation much more easily once it's ready to be incorporated.

For next steps, I think we should do the following:

1. Update this branch with the recent changes to master using the button on this page, and resolve any merge conflicts
2. Add a Tests Added section to the Current Release section in docs/releasehistory.rst acknowledging your contributions and citing the talk where this molecule was discussed
3. Merge the PR
4. Discuss whether you would like to work on building+testing the infrastructure to do bond order interpolation during actual parametrization.

So, let's have a Zoom call some time this week :-)

[1] The two commits did the following

• rename the new molecules from molecule to CID20742535 (the pubchem ID of the structure)
• remove the WBO-calculation call from ToolkitAM1BCCHandler.create_force. The molecule that exists in that function is a temporary copy that only exists during charge assignment, whereas the WBO values would be needed in BondHandler.create_force (which has no knowledge of ToolkitAM1BCCHandler's temp_mol).

[j-wags]

Looks great. Thanks, @sukanyasasmal!