Speeds up coordgen

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.

Also adds a simplistic test that the coordinates generated don't
have crazy bond lengths.

CoordgenMinimizer.cpp

@@ -1263,6 +1263,20 @@ bool CoordgenMinimizer::bondsClash(sketcherMinimizerBond* bond,
         bond->getEndAtom() == bond2->getEndAtom()) {
         return false;
     }
+    auto& start1 = bond->getStartAtom()->coordinates;
+    auto& start2 = bond2->getStartAtom()->coordinates;
+    auto& end1 = bond->getEndAtom()->coordinates;
+    auto& end2 = bond2->getEndAtom()->coordinates;
+    // coincidence and intersection calculations are expensive. Often bonds
+    // are nowhere near each other, so skip the remaining work if a bond is
+    // strictly to the left or right of another bond.
+    if (max(start1.x(), end1.x()) < min(start2.x(), end2.x()) ||
+        max(start1.y(), end1.y()) < min(start2.y(), end2.y()) ||
+        min(start1.x(), end1.x()) > max(start2.x(), end2.x()) ||
+        min(start1.y(), end1.y()) > max(start2.y(), end2.y())) {
+        return false;
+    }
+
     if (sketcherMinimizerMaths::pointsCoincide(
             bond->getStartAtom()->coordinates,
             bond2->getStartAtom()->coordinates) ||

sketcherMinimizerAtom.h

@@ -144,7 +144,7 @@ class EXPORT_COORDGEN sketcherMinimizerAtom
      * stereochemistry */
     bool hasNoStereoActiveBonds() const;
 
-    sketcherMinimizerPointF getCoordinates() const { return coordinates; }
+    const sketcherMinimizerPointF& getCoordinates() const { return coordinates; }
     int getAtomicNumber() const { return atomicNumber; }
 
     void setAtomicNumber(int number) { atomicNumber = number; }

sketcherMinimizerClashInteraction.h

@@ -32,7 +32,7 @@ class sketcherMinimizerClashInteraction : public sketcherMinimizerInteraction
     /* calculate the energy of the clash */
     void energy(float& e) override
     {
-        float squaredDistance =
+        squaredDistance =
             sketcherMinimizerMaths::squaredDistancePointSegment(
                 atom2->coordinates, atom1->coordinates, atom3->coordinates);
         if (squaredDistance > restV)
@@ -49,16 +49,13 @@ class sketcherMinimizerClashInteraction : public sketcherMinimizerInteraction
         energy(totalE);
         if (skipForce)
             return;
+        if (squaredDistance > restV)
+            return;
+
         sketcherMinimizerPointF atomP = atom2->coordinates;
         sketcherMinimizerPointF bondP1 = atom1->coordinates;
         sketcherMinimizerPointF bondP2 = atom3->coordinates;
 
-        float squaredDistance =
-            sketcherMinimizerMaths::squaredDistancePointSegment(atomP, bondP1,
-                                                                bondP2);
-        if (squaredDistance > restV)
-            return;
-
         sketcherMinimizerPointF projection =
             sketcherMinimizerMaths::projectPointOnLine(atomP, bondP1, bondP2);
         sketcherMinimizerPointF f = atomP - projection;
@@ -72,6 +69,8 @@ class sketcherMinimizerClashInteraction : public sketcherMinimizerInteraction
 
     float k2;
     sketcherMinimizerAtom* atom3;
+private:
+    float squaredDistance;
 };
 
 #endif // sketcherMINIMIZERCLASHINTERACTION

test/test_coordgen.cpp

@@ -5,6 +5,7 @@
 #include <boost/test/unit_test.hpp>
 
 #include "../sketcherMinimizer.h"
+#include "../sketcherMinimizerMaths.h"
 
 #include "maeparser/MaeConstants.hpp"
 #include "maeparser/Reader.hpp"
@@ -13,6 +14,41 @@ using namespace schrodinger;
 
 const boost::filesystem::path test_samples_path(TEST_SAMPLES_PATH);
 
+namespace {
+std::map<sketcherMinimizerAtom*, int> getReportingIndices(sketcherMinimizerMolecule& mol) {
+    std::map<sketcherMinimizerAtom*, int> fakeIndices;
+    int index = 0;
+    for (auto& atom : mol.getAtoms()) {
+        fakeIndices.emplace(atom, ++index);
+    }
+    return fakeIndices;
+}
+
+bool areBondsNearIdeal(sketcherMinimizerMolecule& mol, std::map<sketcherMinimizerAtom*, int>& indices)
+{
+    constexpr float targetBondLength = BONDLENGTH*BONDLENGTH;
+    constexpr float tolerance = targetBondLength * 0.1;
+
+    bool passed = true;
+    for (auto& bond : mol.getBonds()) {
+        auto& startCoordinates = bond->getStartAtom()->getCoordinates();
+        auto& endCoordinates = bond->getEndAtom()->getCoordinates();
+
+        const auto sq_distance = sketcherMinimizerMaths::squaredDistance(startCoordinates, endCoordinates);
+        const auto deviation = sq_distance - targetBondLength;
+        if (deviation < -tolerance || deviation > tolerance) {
+            std::cerr << "Bond" << indices[bond->getStartAtom()] << '-'
+                << indices[bond->getEndAtom()] << " has length "
+                << sq_distance << " (" << targetBondLength << ")\n";
+            passed = false;
+        }
+    }
+
+    return false;
+}
+
+}
+
 BOOST_AUTO_TEST_CASE(SampleTest)
 {
     // A small sample test showing how to import a molecule from a .mae file and
@@ -40,4 +76,7 @@ BOOST_AUTO_TEST_CASE(SampleTest)
         // rounding issues depending on platform and environment.
         BOOST_CHECK(c.x() != 0 || c.y() != 0);
     }
+
+    auto indices = getReportingIndices(*mol);
+    BOOST_CHECK(areBondsNearIdeal(*mol, indices));
 }