TinkerTools / tinker

Tinker: Software Tools for Molecular Design
https://dasher.wustl.edu/tinker/
Other
130 stars 61 forks source link

indexing error in potential fitting #87

Closed zjing7 closed 3 years ago

zjing7 commented 3 years ago

There is a bug in potential fitting that I encountered several times where the parameters after fitting have huge errors in ESP. I believe this is because the indices for multipole parameters are shifted when there are some zero-valued parameters. Although Tinker 8.7 has no problem for this molecule, it could be that the final parameters happen to have a small non-zero value.

Attached are the input and output files. tinker87.log, tinker87.key_3 are the results from the older version tinker892.log, tinker892.key_3 are the results from the latest version.

esp.zip

jayponder commented 3 years ago

Hi Francis, This is now fixed, and the fix is pushed to the master Github repo.

The issue was caused by a "regularization" of partial charges and multipole moments that I added to POTENTIAL between 8.7 and 8.8. When a multipole value was initially nonzero, but got optimized to a value below some regularization tolerance of about 0.00001, then it got regularized to zero and dropped from the list of active multipoles during post-processing following the fitting. This mostly only occurs when an X-dipole component probably should be zero by symmetry in the Z-then-X frame, but the DMA gives some very small nonzero value. Sometimes the fitting moves this dipole component even closer to zero, and below the tolerance in the code. In the revised version, I only regularize partial charges, monopoles and diagonal quadrupole components, as these are the values that finally must sum to an integer (charges/monopoles) or to zero (traceless diagonal quadrupoles).

Finally, I would say that many of the cases that cause this problem; ie, Z-then-X local frames where the X-dipole component is very near zero or must be zero by symmetry should actually use Z-Only local frames. Then, this whole problem would go away. But the change made to the code should also handle all or nearly all such cases.

zjing7 commented 3 years ago

The latest version has the same problem for another molecule

Please see the following files in the attachment tinker87.log, tinker87.key_3 tinker892.log, tinker892.key_3 esp-2.zip

Tinker 8.7 also has this problem occasionally. My workaround is to increase RESP-WEIGHT.

jayponder commented 3 years ago

Hi Francis, OK, I've fixed this new failure in the Tinker master Github repo as well... From the POTENTIAL output, this new case looks like the same error as the first one you reported above. But actually it's somewhat different.

The first error was due to X-dipole components that were very small in the DMA, and then become zero and were dropped out during the potential fitting when using a Z-then-X local frame. In at least some of these cases, the local frame should be Z-Only with the X-dipole always exactly zero. But the code should now handle things correctly even if the local frame is treated as Z-then-X.

The new error is due to two diagonal quadrupole components (XX- and YY-) starting out as different in the DMA, and then becoming exactly equal during potential fitting. That can also happen when the site should have been a Z-Only local frame, where XX- and YY-quadrupole components must be equal. But in the case you provided, I think the XX- and YY- quadrupoles just become equal to within a tolerance of 0.00001 by "accident". I've now patched the code so that these quadrupole cases are also handled.

I'm going to close this issue again. But obviously let me know if you find another similar case that fails.