Parameter assignment regression test for FreeSolv fails due bug in toolkit version 0.0.4

[andrrizzi]

I've attempted to test whether the current toolkit can recreate the Systems we used for the hydration free energy calculations in the SMIRNOFF paper. Currently, the test fails for molecule 1019269 (1-butanol). Because of a different Lennard-Jones parameter that changed from version 0.0.3 (the toolkit version used for the calculations) to the current smirnoff99Frosst.offxml. Here are the parameters:

• 0.0.3 (id n13): https://github.com/openforcefield/openforcefield/blob/0.0.3/openforcefield/data/forcefield/smirnoff99Frosst.ffxml#L316
• 0.1 (id n12): https://github.com/openforcefield/openforcefield/blob/0.1.0/openforcefield/data/forcefield/smirnoff99Frosst.offxml#L315

Is this expected? If it is, should I just ignore the test case?

[andrrizzi]

Actually, I'm not sure that's the problem because the error I get is:

The following particles have different parameters in the two NonbondedForce forces for the current OpenFF and SMIRNOFF 0.0.3 systems respectively:
particle 14:
    epsilon: 1.2552 kJ/mol != 0.0002204968 kJ/mol
    sigma: 9.390072489199176e-06 nm != 0.053453923088420355 nm

and the SMIRNOFF 0.0.3 parameters are different from the file I linked above. I'm assuming 0.03 files was used according to this README.md, but I'll check whether there's no error in reading the topology or similar.

[jchodera]

Neither of those could possibly be right!

0.0.3 has zero epsilon and sigma, which we shouldn't allow:

     <Atom smirks="[#1:1]-[#8]" epsilon="0.0000" id="n13" rmin_half="0.0000"/>

and 0.1 has an rmin_half that is WAY too small for the epsilon:

    <Atom smirks="[#1:1]-[#8]" epsilon="0.3000" id="n12" rmin_half="5.27e-05"/>

[andrrizzi]

After a little, I've noticed that

>>> (0.0002204968 * u.kilojoule_per_mole).value_in_unit(u.kilocalories_per_mole)
5.27e-05  # which is the same value of rmin_half in 0.1
>>> (0.053453923088420355 * u.nanometers).value_in_unit(u.angstrom) / 2 * 2**(1/6)
0.3  # which is the same value of epsilon in 0.1

so I think the repeated hydration free energy calculations were set up using 0.0.4, not 0.0.3, which has the same parameters for n12 as 0.1, but somehow the code inverted the two values when assigning the NonbondedForce parameters.

I'll run the same check tomorrow on the full freesolv set to check the extent of the problem.

[andrrizzi]

I've run the parameter assignment test on all the FreeSolv set with the OpenMM XML files generated for the SMIRNOFF 0.1 hydration free energy calculations, and it failed for 126/517 molecules.

I've opened a temporary branch test-freesolv with the test code and a representative subset of molecules that will generate all the errors I've found. You can reproduce the error messages by checking out that branch and running from the main folder of the project

pytest openforcefield/tests/test_forcefield.py::test_freesolv_parameters_assignment

Errors description

Here is a summary (freesolv molecule ID -> failure description):

1. 1019269: This fails because of the error I described above in 0.0.4. It seems like the epsilon and rmin_half parameters were switched before assignment. The problem was fixed in Fix human error in order of sigma/epsilon for polar hydrogens. #179 .
2. 1723043: Fails because in 0.0.2, a7 ([#6r4:1]-;@[#6r4:2]-;@[#6r4:3]) and a8 ([!#1:1]-[#6r4:2]-;!@[!#1:3]) matches were assigned to some other angle with k="100.0" and angle="109.5", which was probably a1 ([*:1]~[#6X4:2]-[*:3]). The current a7 and a8 were not present in the 0.0.3 toolkit.
3. 6266306: Fails because in 0.0.2, a a9 ([!#1:1]-[#6r4:2]-;!@[#1:3]) match was assigned to some other angle with k="100.0" and angle="109.5", again, probably a1. The current a9 was not present in the 0.0.3 toolkit.
4. 3323117 (SMILES: [H]C1(C(C([S+2](C1([H])[H])([O-])[O-])([H])[H])([H])[H])[H]): Fails because in 0.0.2, sometimes a b56 ([#16X4,#16X3:1]~[#8X1:2]) match was assigned to b55 ([#16X4,#16X3:1]-[#8X2:2]) instead.
5. 63712 (SMILES: [H]C1(C(C(N(C(C1([H])[H])([H])[H])C([H])([H])[H])([H])[H])([H])[H])[H]): Fails because in 0.0.2, a t51 ([*:1]-[#6X4:2]-[#7X3:3]-[*:4]) match was assigned to some other proper torsion with k1="0.156" and same periodicity 3 (likely t50 [*:1]-[#6X4:2]-[#7X4:3]-[*:4] or t56 [!1:1]-[#7X4,#7X3:2]-[#6X4;r3:3]-[*:4]).

On top of these errors, I had 4 molecules that gave some problems to the OpenEye toolkit wrapper.

• 820789: Fails with "Exception: Unable to assign charges".
• 2501588, 3266352, 7829570: Fail with UndefinedStereochemistryError.

What to do now?

By manual inspection, it seems to me that the current version of the toolkit is doing the right thing in assigning the parameters (although a second pair of eyes would be helpful). It seems to me, the old toolkit had problems with bond orders and/or ring matches. If this is true, we need to decide

1. What to do about tests here?
2. What to do about the calculations that used molecules affected by those problems.

For 1., we could ignore these molecules and maybe regenerate a representative subset of them with the current version of the toolkit for future regression tests. I'm not sure it's worth spending some time debugging the old version of the toolkit at this point, although this may affect the current stable version (i.e. the one in master, not the one in the topology branch).

[jchodera]

I think it is worth figuring out what went wrong here in terms of whether the failure could affect the current version of the toolkit. In particular, is this a problem with

1. the way the old openforcefield toolkit handled matching logic
2. the cheminformatics toolkit SMARTS matching
3. the way that we have written the SMARTS strings

I don't think we need to fix the old toolkit---which would actually change the behavior we would need to reproduce the old data, and wouldn't be very useful to do---but it would be good to identify the root cause(s).

[andrrizzi]

I manually checked the molecule topologies for the examples above and it looked to me that the current toolkit (in topology) is matching the correct SMARTS, but if @bannanc or @davidlmobley had time to double-check, I think that would be helpful.

I'll work to make sure that all the other failures are caused by the same problems (and don't just result in the same error).

[davidlmobley]

@jchodera @andrrizzi relating to the specific rmin_half/epsilon for polar hydrogens noted here:

<Atom smirks="[#1:1]-[#8]" epsilon="0.3000" id="n12" rmin_half="5.27e-05"/>

Cross-posting from Slack:

the specific parameter you just cited is the “nonzero sigma” for polar hydrogens that I added to smirnoff99Frosst. All AMBER force fields use a sigma of zero, but this causes severe problems for some systems (see the SMIRNOFF paper) so I made a very small perturbation to the sigma designed in a specific way (detailed there) to eliminate the problems while also not perturbing densities, etc. in any significant way. That’s where your polar hydrogen parameter with “odd … sigma” is coming from

Andrea thinks this is a swap of sigma and epsilon. I'll have to check; that doesn't sound right ot me.

[davidlmobley]

@andrrizzi - I think one of your tests catches an actual bug in the forcefield (fixed in #179 ). The others I'm less sure about and will require @bannanc . We DID catch a couple of parameter issues that we concluded needed fixing since the paper was put out, so it could be that the test SHOULD fail for these specific cases. @bannanc will need to investigate/weigh in; I'll send this to her on Slack also.

[davidlmobley]

@andrrizzi also check out openforcefield/smirnoff99Frosst#85 which may be one of the changes you're running into.

[davidlmobley]

See also openforcefield/smirnoff99Frosst#81 which is another.

I think these are the only two changes. I'm not seeing a change to torsions.

[bannanc]

@andrrizzi I'm trying to understand the problems you posted so I'm going to comment on them one at a time:

• 820789: Fails with "Exception: Unable to assign charges".
• 2501588, 3266352, 7829570: Fail with UndefinedStereochemistryError.

Are you using the same input files and version of openeye as was used before? It seems weird for these to have not been a problem before, but are a problem now.

• 1019269: This fails because of the error I described above in 0.0.4. It seems like the epsilon and rmin_half parameters were switched before assignment.

I think we've handled this case as an intentional change in the parameters, as discussed above, it is a typo from what we actually wanted, but was an officially released version of the force field.

• 1723043: Fails because in 0.0.4, a7 ([#6r4:1]-;@[#6r4:2]-;@[#6r4:3]) and a8 ([!#1:1]-[#6r4:2]-;!@[!#1:3]) matches were assigned to some other angle with k="100.0" and angle="109.5", which was probably a1 ([*:1]~[#6X4:2]-[*:3]).

I'm having a hard time understanding the direction of this, but in version 1.0.6 we added cyclobutyl parameters that were not there before. So if you're saying this [*:1]~[#6X4:2]-[*:3] parameter used to be applied and now a7 and a8 are instead, then I think this is correct. However, if it is the reverse then we have a problem.

• 6266306: Fails because in 0.0.4, a a9 ([!#1:1]-[#6r4:2]-;!@[#1:3]) match was assigned to some other angle with k="100.0" and angle="109.5", again, probably a1.

OK, I think this is the same change in parameters as above, but it seems like you should always get an intracyclic and extracyclic parameter change so I don't understand how you're not seeing a7 here. It would be really helpful if you could include SMILES or pictures of the molecules when you highlight issues so I can understand what chemistry is there.

• 3323117: Fails because in 0.0.4, sometimes a b56 ([#16X4,#16X3:1]~[#8X1:2]) match was assigned to b55 ([#16X4,#16X3:1]-[#8X2:2]) instead.

This one makes me worry you have a different tautomer or something with your molecule because the first SMIRKS should only match a sulfur next two a monovalent oxygen (#8X1) and the second a divalent oxygen (#8X2). We haven't changed those parameters and they should never match the same bond because the chemistry for those SMIRKS are different. So again, are you sure you're using the same inputs for both tests?

• 63712: Fails because in 0.0.4, a t51 ([*:1]-[#6X4:2]-[#7X3:3]-[*:4]) match was assigned to some other proper torsion with k1="0.156" and same periodicity 3 (likely t50 [*:1]-[#6X4:2]-[#7X4:3]-[*:4] or t56 [!1:1]-[#7X4,#7X3:2]-[#6X4;r3:3]-[*:4]).

OK, I'm still not clear on how you're identifying which parameter is being assigned incorrectly... I don't remember us changing this torsion, so we'd need to dig a little deeper. However, you should consider the chemistry of the center bond, t50 cannot match the same bond as t51, note the change in the connectivity on the nitrogen. However, t56 is a more subset of the chemistry matched in t51, same connectivity on the carbon and nitrogen, but now the carbon is in a 3-membered ring.

[bannanc]

If the goal is to check that the parameters are assigned in the same way, I think it makes more sense to look at which SMIRKS match the molecule, rather than trying to understand where the energy difference tracks to a parameter. I did this when we switched from the R decorator to x for agreement with RDkit, you should be able to do something similar with each version of the code storing a dictionary of atom indices mapping to SMIRKS assigned. Here is the jupyter notebook I used to do that comparison. Obviously this step is more complex since that was the same version of openforcefield with just different offxml files.

[andrrizzi]

Thank you both for the pointers, @bannanc, @davidlmobley!

I think that clears up the issue with the nonbonded and angle parameters. I'll update my summary post above accordingly (and my testing code).

Are you using the same input files and version of openeye as was used before? It seems weird for these to have not been a problem before, but are a problem now.

I think I'm using the same input files, but a more recent version of OpenEye. I'm not sure about the charge error, but I think the UndefinedStereochemistryError may be caused the new openforcefield toolkit, which I believe is more strict about stereochemistry than what it used to. I'll check if anything changes with the old OpenEye anyway, thanks!

if you're saying this [*:1]~[#6X4:2]-[*:3] parameter used to be applied and now a7 and a8 are instead, then I think this is correct

Great! That's exactly what is happening.

It would be really helpful if you could include SMILES or pictures of the molecules

Sorry about that! I thought the mol2 files I uploaded were sufficient but I now realize it's not the most convenient way of sharing. I'll update my post above with the SMILES!

I'm still not clear on how you're identifying which parameter is being assigned incorrectly

In short, I'm taking the OpenMM XML System input files we generated for the hydration free energies (for example, see here: https://github.com/MobleyLab/SMIRNOFF_paper_code/tree/master/FreeSolv_repeat_subset), then reparameterizing those systems starting from the same input mol2 files with the latest openforcefield toolkit and check that it assigns the same parameters.

To guess the possible match that the old toolkit found, I was just looking at the old smirnoff99Frosst.ffxml files and search for the SMIRKS associated to the different parameters, but I haven't confirmed those yet.

If the goal is to check that the parameters are assigned in the same way, I think it makes more sense to look at which SMIRKS match the molecule, rather than trying to understand where the energy difference tracks to a parameter.

The goal here was to add a generic Travis test to make sure we didn't introduce any major regression in the parameter assignment in the topology branch. I'm planning to do exactly that for debugging, but, as you can imagine, it would be trick it to implement it as a Travis test since it will require multiple versions of the toolkit. Thanks for the notebook! Will take a look shortly.

[andrrizzi]

See also openforcefield/smirnoff99Frosst#81 which is another.

I'm probably missing something, but I'm not sure this change made it into the openforcefield toolkit. In the latest version of the smirnoff99Frosst.offxml, the parameters for [#6X2:1]-[#1:2] and [#6X4:1]-[#1:2] look still identical.

[bannanc]

@andrrizzi Are you using the the smirnoff99Frosst.offxml in the master branch? We've made changes to smirnoff99Frosst since the topology branch was created? Those parameters are changed in the openforcefield master

[andrrizzi]

Ah! That's it, thank you! I'm planning to merge master into topology right after #179 gets merged so that should fix it.

[davidlmobley]

Thanks, @bannanc and @andrrizzi !

I've also adjusted my github notification preferences so hopefully I will be more responsive when tagged now. (Before I was getting deluged with so many notifications I was missing the ones where I was tagged.)

[andrrizzi]

I think I've tracked down the origin of the failures for the bond and the proper torsions.

The problem affects 9 molecules of the FreeSolv set used for the first batch of the hydration free energy calculations (not the repeat with the update polar hydrogen parameters), which were parametrized with openforcefield 0.0.2.

The different behavior in 0.0.2 is triggered by incorrect information in the mol2 file about the bond order and/or this line, which calls oechem.OEAssignImplicitHydrogens(oemol) before searching for SMARTS matches. This would cause some atom valences to be misinterpreted during SMARTS searches.

Here are two examples with more details:

3323117

The S=O bond in the figure above should match [#16X4,#16X3:1]~[#8X1:2], but the OEBond object after loading the mol2 file with OpenEye has bond order 1 and calling OEAssignImplicitHydrogens caused the bond to match [#16X4,#16X3:1]-[#8X2:2] instead in version 0.0.2.

63712

The torsion involving the tetrahedral nitrogen above should match [*:1]-[#6X4:2]-[#7X3:3]-[*:4], but calling OEAssignImplicitHydrogens caused the torsion to match [*:1]-[#6X4:2]-[#7X4:3]-[*:4] in 0.0.2.

Here is the complete list of molecules affected by the problem

• 3323117
• 63712
• 1944394
• 2771569
• 4059279
• 5282042
• 8449031
• 8558116
• 9209581

3323117 is the only one with the bond problem, all the others have the same torsion issue involving a tetrahedral nitrogen.

The call to OEAssignImplicitHydrogens was removed in the 0.0.3 version (see here) so the problem should not affect openforcefield >= 0.0.3.

I am going to investigate the charged and undefined stereochemistry errors next.

[jchodera]

Wow---great sleuthing, @andrrizzi! That was indeed an important bugfix.

[andrrizzi]

I believe the remaining problems are not worrisome.

The Exception: Unable to assign charges error found with 820789 was just the result of a different charge engine and we're not comparing charges anyway.

I think the UndefinedStereochemistryError raised with the current version (these checks were not implemented in 0.0.2) with 2501588, 3266352, and 7829570 are being conservative. OpenEye cannot figure out the chirality of these nitrogens:

2501588: N-(cyclopropylmethyl)-2,6-dinitro-N-propyl-4-(trifluoromethyl)aniline

3266352: N-methyl-N-(2,2,2-trifluoroethyl)aniline

7829570: N-butyl-N-ethyl-2,6-dinitro-4-(trifluoromethyl)aniline

I believe in all cases the lone pair is delocalized and the nitrogen is not chiral, but it would be helpful if somebody else could double-check these.

[jchodera]

This may be relevant to @davidlmobley's work on tertiary amines and @ChayaSt's findings on OpenEye perception of these atoms as chiral.

[davidlmobley]

Thanks, @andrrizzi -- I agree with your points and what you've done here. Excellent.

And this is not relevant immediately to our work on tertiary amines, at least in the sense that we haven't been doing anything with these molecules.

[j-wags]

The issue here seems to be that:

• The geometry indicates to OE the nitrogen is planar.
• The bond orders indicate to OE that the nitrogen should be tetrahedral
  • The mol2 shows all bonds being "1", but I think the planar geometry indicates at least one should be "ar" or "1.5" (forgot the exact spec).

So, beyond this specific case, what is the desired behavior when a user gives a molecule where geometry and bond order disagree`?

[jchodera]

So, beyond this specific case, what is the desired behavior when a user gives a molecule where geometry and bond order disagree`?

I believe we should only be using the geometry information for assigning partial charges right now. However, this should later be corrected, since it would be ideal if we had a reproducible charging scheme that produced charges that were entirely independent of conformation.

For typing, all information should come from the chemical information in the Molecule (without reference to any geometry information).

Ultimately, we should aim to produce force field parameters that only utilize the chemical information in the Molecule (without any geometry information) for all force field parameters we produce.

While it may be useful to have some utilities that check for consistency between geometry-derived information (bond orders, chirality, etc) and the direct chemical information specified in the Molecule, extracting this chemical information from the geometry is an imperfect art based on heuristics, and may be so frequently problematic as to not be helpful.

[davidlmobley]

I agree with John entirely: The semiempirical QM calculation should use the geometry; the typing/chemical perception should use the molecule. If these are inconsistent, we might eventually want something on the front end which will warn people about this (there is an issue discussing this which is open somewhere).

To rehash what's in that issue, it's likely that we'll have two main categories of users:
a) "Expert" or "professional" users (often pharma) who will want to be able to put in a (often well curated) molecule and, potentially, geometry and just have them "work" without us messing with things or warning them, and
b) Non-expert/rookie users (often early-stage students in academia, but others too) who will put in molecules they got from who-knows-where and need all the help they can get to keep from shooting themselves in the foot or worse

This means basically we want to warn everyone when they do something which might be questionable/odd, and allow people in category (b) to easily take a chemistry "clean up" pass before bringing in their molecules. But people in (a) won't want that.

[j-wags]

@jchodera

I believe we should only be using the geometry information for assigning partial charges right now.

We currently use geometry to get stereochemistry when loading molecules from 3D representations.

We need to perceive stereochemistry when making OFFMols if we want to keep the door open for stereo-specific SMARTS.

We might want to punt on this. However, it's a big decision. If we don't build infrastructure for strict stereochemistry checking now, the day that we update the toolkit to make those checks (maybe we release a FF version with stereo-specific SMARTS), it will start complaining about loading molecules that it was previously just fine with.

[andrrizzi]

This issue was resolved in #183 .