Merge pull request #896 from aerkiaga/neighbor-perceiver-generalize

Fix NeighborPerceiver not handling coordinates below the minimum

avogadro/core/neighborperceiver.cpp

@@ -37,20 +37,20 @@ NeighborPerceiver::NeighborPerceiver(const Array<Vector3> points, float maxDista
   );
   m_bins = bins;
   for (Index i = 0; i < points.size(); i++) {
-    std::array<size_t, 3> bin_index = getBinIndex(points[i]);
+    std::array<int, 3> bin_index = getBinIndex(points[i]);
     m_bins.at(bin_index[0]).at(bin_index[1]).at(bin_index[2]).push_back(i);
   }
 }
 
 const Array<Index> NeighborPerceiver::getNeighbors(const Vector3 point) const
 {
   Array<Index> r;
-  const std::array<size_t, 3> bin_index = getBinIndex(point);
-  for (size_t xi = std::max(size_t(1), bin_index[0]) - 1;
+  const std::array<int, 3> bin_index = getBinIndex(point);
+  for (int xi = std::max(int(1), bin_index[0]) - 1;
       xi < std::min(m_binCount[0], bin_index[0] + 2); xi++) {
-    for (size_t yi = std::max(size_t(1), bin_index[1]) - 1;
+    for (int yi = std::max(int(1), bin_index[1]) - 1;
         yi < std::min(m_binCount[1], bin_index[1] + 2); yi++) {
-      for (size_t zi = std::max(size_t(1), bin_index[2]) - 1;
+      for (int zi = std::max(int(1), bin_index[2]) - 1;
           zi < std::min(m_binCount[2], bin_index[2] + 2); zi++) {
         std::vector<Index> bin = m_bins[xi][yi][zi];
         r.insert(r.end(), bin.begin(), bin.end());
@@ -60,9 +60,9 @@ const Array<Index> NeighborPerceiver::getNeighbors(const Vector3 point) const
   return r;
 }
 
-const std::array<size_t, 3> NeighborPerceiver::getBinIndex(const Vector3 point) const
+const std::array<int, 3> NeighborPerceiver::getBinIndex(const Vector3 point) const
 {
-  std::array<size_t, 3> r;
+  std::array<int, 3> r;
   for (size_t c = 0; c < 3; c++) {
     r[c] = std::floor((point(c) - m_minPos(c)) / m_maxDistance);
   }

avogadro/core/neighborperceiver.h

@@ -43,10 +43,10 @@ class AVOGADROCORE_EXPORT NeighborPerceiver
   const Array<Index> getNeighbors(const Vector3 point) const;
 
 private:
-  const std::array<size_t, 3> getBinIndex(const Vector3 point) const;
+  const std::array<int, 3> getBinIndex(const Vector3 point) const;
 protected:
   float m_maxDistance;
-  std::array<size_t, 3> m_binCount;
+  std::array<int, 3> m_binCount;
   std::vector<std::vector<std::vector<std::vector<Index>>>> m_bins;
   Vector3 m_minPos;
   Vector3 m_maxPos;

tests/core/CMakeLists.txt

@@ -14,6 +14,7 @@ set(tests
   Mesh
   Molecule
   Mutex
+  NeighborPerceiver
   RingPerceiver
   Spacegroup
   Utilities

tests/core/neighborperceivertest.cpp

@@ -0,0 +1,61 @@
+/******************************************************************************
+  This source file is part of the Avogadro project.
+  This source code is released under the 3-Clause BSD License, (see "LICENSE").
+******************************************************************************/
+
+#include <gtest/gtest.h>
+
+#include <avogadro/core/array.h>
+#include <avogadro/core/neighborperceiver.h>
+#include <avogadro/core/vector.h>
+
+using Avogadro::Core::Array;
+using Avogadro::Core::NeighborPerceiver;
+using Avogadro::Vector3;
+
+TEST(NeighborPerceiverTest, positive)
+{
+  Array<Vector3> points;
+  points.push_back(Vector3(0.0, 0.0, 0.0));
+  points.push_back(Vector3(1.0, 0.0, 0.0));
+  points.push_back(Vector3(0.0, 1.5, 1.5));
+  points.push_back(Vector3(2.1, 0.0, 0.0));
+
+  NeighborPerceiver perceiver(points, 1.0f);
+
+  auto neighbors = perceiver.getNeighbors(Vector3(0.0, 0.0, 0.0));
+  EXPECT_EQ(neighbors.size(), static_cast<size_t>(3));
+}
+
+TEST(NeighborPerceiverTest, negative)
+{
+  Array<Vector3> points;
+  points.push_back(Vector3(0.0, 0.0, 0.0));
+  points.push_back(Vector3(-1.0, 0.0, 0.0));
+  points.push_back(Vector3(0.0, -1.5, -1.5));
+  points.push_back(Vector3(-2.1, 0.0, 0.0));
+
+  NeighborPerceiver perceiver(points, 1.0f);
+
+  auto neighbors = perceiver.getNeighbors(Vector3(0.0, 0.0, 0.0));
+  EXPECT_EQ(neighbors.size(), static_cast<size_t>(3));
+}
+
+TEST(NeighborPerceiverTest, bounds)
+{
+  Array<Vector3> points;
+  points.push_back(Vector3(0.0, 0.0, 0.0));
+
+  NeighborPerceiver perceiver(points, 1.0f);
+
+  auto neighbors = perceiver.getNeighbors(Vector3(0.0, 0.0, 0.0));
+  EXPECT_EQ(neighbors.size(), static_cast<size_t>(1));
+  neighbors = perceiver.getNeighbors(Vector3(1.5, 0.0, 0.0));
+  EXPECT_EQ(neighbors.size(), static_cast<size_t>(1));
+  neighbors = perceiver.getNeighbors(Vector3(2.5, 0.0, 0.0));
+  EXPECT_EQ(neighbors.size(), static_cast<size_t>(0));
+  neighbors = perceiver.getNeighbors(Vector3(-0.5, 0.0, 0.0));
+  EXPECT_EQ(neighbors.size(), static_cast<size_t>(1));
+  neighbors = perceiver.getNeighbors(Vector3(-1.5, 0.0, 0.0));
+  EXPECT_EQ(neighbors.size(), static_cast<size_t>(0));
+}