Closed d-b-w closed 5 years ago
oh, bummer. It looks like the sketcherMinimizerClashInteraction
objects are reused, sometimes not even with the same atoms. Caching the distance in the sketcherMinimizerClashInteraction
leads to incorrect results, though fewer problems than I would have expected for the cost.
I'll back out that part of the change.
I pulled out the refactorings I was doing to try to understand things into a separate set of commits. This is now only the performance improvement, a test, and a change to remove some unused headers to speed up my rebuilds.
@d-b-w, the CI builds failed, apparently, because of an unused variable (travis) and some constexpr
s (appveyor).
Thanks, @ricrogz . I see 'em.
Improve coordgen performance: #39
When detecting clashes, do a rough binning of atoms. If both atoms for one bond are above both atoms of the other bond, they can't be a clash. This saves clash calculations for things that are obviously not clashes. It has an impact on big structures that are partially templated. (overall 15% speedup, but 40% improvement for the worst molecules in the test case.
coordgen is still slower than RDkit's native coordinate generation, though.
Also adds a simple test that the coordinates generated don't have crazy bond lengths. This would have caught my earlier misunderstanding.