Minimal C++ replacement for rvec
authorTeemu Murtola <teemu.murtola@gmail.com>
Sun, 14 Dec 2014 14:19:11 +0000 (16:19 +0200)
committerGerrit Code Review <gerrit@gerrit.gromacs.org>
Mon, 5 Jan 2015 22:30:24 +0000 (23:30 +0100)
Add a C++ class that acts as rvec, but also supports placement in STL
containers (i.e., is default-constructible, copy-constructible, and
assignable) and can be returned from functions.  There is some ugly
reinterpret_casting required for rvec arrays to make this work, which
requires that the compiler generates compatible layouts for the class
and rvec.  The tests ensure that this is the case, but it could be nice
to check some more exotic compilers as well, unless someone knows the
C/C++ standards well enough to say that this layout is always ensured.

Passing RVec as 'const rvec &' unfortunately also needs an explicit
function call because of issues with cppcheck and MSVC.

Minimal solution to #1017.

Change-Id: I79be58bf7be2a78f787f470044b3b5abf234ec0c

src/gromacs/math/CMakeLists.txt
src/gromacs/math/tests/CMakeLists.txt [new file with mode: 0644]
src/gromacs/math/tests/vectypes.cpp [new file with mode: 0644]
src/gromacs/math/vectypes.h
src/gromacs/selection/nbsearch.cpp
src/gromacs/selection/nbsearch.h
src/gromacs/selection/tests/nbsearch.cpp
src/gromacs/trajectoryanalysis/modules/angle.cpp
src/gromacs/trajectoryanalysis/tests/surfacearea.cpp

index 94c71d45a3d78d1cfbcd8f1e9b0bb5cdcda1c3bf..d05cf8016471f7f7145653ea14564077155652d0 100644 (file)
@@ -47,5 +47,5 @@ gmx_install_headers(
     )
 
 if (BUILD_TESTING)
-#    add_subdirectory(tests)
+    add_subdirectory(tests)
 endif()
diff --git a/src/gromacs/math/tests/CMakeLists.txt b/src/gromacs/math/tests/CMakeLists.txt
new file mode 100644 (file)
index 0000000..867e28e
--- /dev/null
@@ -0,0 +1,37 @@
+#
+# This file is part of the GROMACS molecular simulation package.
+#
+# Copyright (c) 2014, by the GROMACS development team, led by
+# Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
+# and including many others, as listed in the AUTHORS file in the
+# top-level source directory and at http://www.gromacs.org.
+#
+# GROMACS is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public License
+# as published by the Free Software Foundation; either version 2.1
+# of the License, or (at your option) any later version.
+#
+# GROMACS is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with GROMACS; if not, see
+# http://www.gnu.org/licenses, or write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+#
+# If you want to redistribute modifications to GROMACS, please
+# consider that scientific software is very special. Version
+# control is crucial - bugs must be traceable. We will be happy to
+# consider code for inclusion in the official distribution, but
+# derived work must not be called official GROMACS. Details are found
+# in the README & COPYING files - if they are missing, get the
+# official version at http://www.gromacs.org.
+#
+# To help us fund GROMACS development, we humbly ask that you cite
+# the research papers on the package. Check out http://www.gromacs.org.
+
+gmx_add_unit_test(MathUnitTests math-test
+                  vectypes.cpp
+                  )
diff --git a/src/gromacs/math/tests/vectypes.cpp b/src/gromacs/math/tests/vectypes.cpp
new file mode 100644 (file)
index 0000000..e1ddb19
--- /dev/null
@@ -0,0 +1,134 @@
+/*
+ * This file is part of the GROMACS molecular simulation package.
+ *
+ * Copyright (c) 2014, by the GROMACS development team, led by
+ * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
+ * and including many others, as listed in the AUTHORS file in the
+ * top-level source directory and at http://www.gromacs.org.
+ *
+ * GROMACS is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1
+ * of the License, or (at your option) any later version.
+ *
+ * GROMACS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with GROMACS; if not, see
+ * http://www.gnu.org/licenses, or write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+ *
+ * If you want to redistribute modifications to GROMACS, please
+ * consider that scientific software is very special. Version
+ * control is crucial - bugs must be traceable. We will be happy to
+ * consider code for inclusion in the official distribution, but
+ * derived work must not be called official GROMACS. Details are found
+ * in the README & COPYING files - if they are missing, get the
+ * official version at http://www.gromacs.org.
+ *
+ * To help us fund GROMACS development, we humbly ask that you cite
+ * the research papers on the package. Check out http://www.gromacs.org.
+ */
+/*! \internal \file
+ * \brief
+ * Tests various corners of gmx::RVec implementation.
+ *
+ * The main point of these tests is to check that all different constructs
+ * using gmx::RVec compile, and that some of the non-trivial conversions
+ * to/from rvec work as intended.
+ *
+ * \author Teemu Murtola <teemu.murtola@gmail.com>
+ * \ingroup module_math
+ */
+#include "gmxpre.h"
+
+#include "gromacs/math/vectypes.h"
+
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include "gromacs/math/vec.h"
+
+namespace
+{
+
+using gmx::RVec;
+
+TEST(RVecTest, CanBeStoredInVector)
+{
+    std::vector<RVec> v;
+    v.push_back(RVec(1, 2, 3));
+    v.resize(2);
+    EXPECT_EQ(1, v[0][XX]);
+    EXPECT_EQ(2, v[0][YY]);
+    EXPECT_EQ(3, v[0][ZZ]);
+}
+
+TEST(RVecTest, ConvertsImplicitlyFrom_rvec)
+{
+    std::vector<RVec> v;
+    rvec              x = { 1, 2, 3 };
+    v.push_back(x);
+    EXPECT_EQ(1, v[0][XX]);
+    EXPECT_EQ(2, v[0][YY]);
+    EXPECT_EQ(3, v[0][ZZ]);
+}
+
+TEST(RVecTest, ConvertsImplicitlyTo_rvec)
+{
+    std::vector<RVec> v;
+    v.push_back(RVec(1, 2, 3));
+    rvec              x;
+    copy_rvec(v[0], x);
+    EXPECT_EQ(1, x[XX]);
+    EXPECT_EQ(2, x[YY]);
+    EXPECT_EQ(3, x[ZZ]);
+}
+
+TEST(RVecTest, WorksAsMutable_rvec)
+{
+    std::vector<RVec> v;
+    v.push_back(RVec(1, 2, 3));
+    rvec              x = {2, 3, 4};
+    copy_rvec(x, v[0]);
+    EXPECT_EQ(2, v[0][XX]);
+    EXPECT_EQ(3, v[0][YY]);
+    EXPECT_EQ(4, v[0][ZZ]);
+}
+
+TEST(RVecTest, WorksAs_rvec_Array)
+{
+    std::vector<RVec> v;
+    v.push_back(RVec(1, 2, 3));
+    v.push_back(RVec(2, 3, 4));
+    const rvec *r = as_rvec_array(&v[0]);
+    EXPECT_EQ(1, r[0][XX]);
+    EXPECT_EQ(2, r[0][YY]);
+    EXPECT_EQ(3, r[0][ZZ]);
+    EXPECT_EQ(2, r[1][XX]);
+    EXPECT_EQ(3, r[1][YY]);
+    EXPECT_EQ(4, r[1][ZZ]);
+}
+
+/*! \brief
+ * Helper function for testing RVec to rvec conversions.
+ */
+const rvec *testFunction(const rvec &x)
+{
+    return &x;
+}
+
+TEST(RVecTest, WorksAs_rvec_Reference)
+{
+    RVec        v(1, 2, 3);
+    const rvec *r = testFunction(v.as_vec());
+    EXPECT_EQ(1, r[0][XX]);
+    EXPECT_EQ(2, r[0][YY]);
+    EXPECT_EQ(3, r[0][ZZ]);
+}
+
+} // namespace
index ebb12dc512f3080576d7f2a18fbef834253ce02d..91b6d7c9a85102b32b8d75aaa86e8c2ca0ea7334 100644 (file)
@@ -56,4 +56,119 @@ typedef int     ivec[DIM];
 
 typedef int     imatrix[DIM][DIM];
 
+#ifdef __cplusplus
+
+namespace gmx
+{
+
+/*! \brief
+ * C++ class for 3D vectors.
+ *
+ * \tparam ValueType  Type
+ *
+ * This class provides a C++ version of rvec/dvec/ivec that can be put into STL
+ * containers etc.  It is more or less a drop-in replacement for `rvec` and
+ * friends: it can be used in most contexts that accept the equivalent C type.
+ * However, there are two cases where explicit conversion is necessary:
+ *  - An array of these objects needs to be converted with as_vec_array() (or
+ *    convenience methods like as_rvec_array()).
+ *  - Passing an RVec as a `const rvec &` parameter to a function needs an
+ *    explicit call to as_vec().  The implicit conversion should work for this
+ *    as well, but cppcheck parses the necessary implicit conversion operator
+ *    incorrectly and MSVC fails to compile code that relies on the implicit
+ *    conversion, so the explicit method is necessary.
+ *
+ * For the array conversion to work, the compiler should not add any extra
+ * alignment/padding in the layout of this class;  that this actually works as
+ * intended is tested in the unit tests.
+ *
+ * \inpublicapi
+ */
+template <typename ValueType>
+class BasicVector
+{
+    public:
+        //! Underlying raw C array type (rvec/dvec/ivec).
+        typedef ValueType RawArray[DIM];
+
+        //! Constructs default (uninitialized) vector.
+        BasicVector() {}
+        //! Constructs a vector from given values.
+        BasicVector(ValueType x, ValueType y, ValueType z)
+        {
+            x_[XX] = x;
+            x_[YY] = y;
+            x_[ZZ] = z;
+        }
+        /*! \brief
+         * Constructs a vector from given values.
+         *
+         * This constructor is not explicit to support implicit conversions
+         * that allow, e.g., calling `std::vector<RVec>:``:push_back()` directly
+         * with an `rvec` parameter.
+         */
+        BasicVector(const RawArray x)
+        {
+            x_[XX] = x[XX];
+            x_[YY] = x[YY];
+            x_[ZZ] = x[ZZ];
+        }
+        //! Indexing operator to make the class work as the raw array.
+        ValueType &operator[](int i) { return x_[i]; }
+        //! Indexing operator to make the class work as the raw array.
+        ValueType operator[](int i) const { return x_[i]; }
+        // The conversion functions below could more accurately return
+        // RawArray &, but this fails with cppcheck and does not solve the
+        // issue with MSVC, so as_vec() should be used instead.
+        //! Makes BasicVector usable in contexts where a raw C array is expected.
+        operator ValueType *() { return x_; }
+        //! Makes BasicVector usable in contexts where a raw C array is expected.
+        operator const ValueType *() const { return x_; }
+
+        //! Converts to a raw C array where implicit conversion does not work.
+        RawArray &as_vec() { return x_; }
+        //! Converts to a raw C array where implicit conversion does not work.
+        const RawArray &as_vec() const { return x_; }
+
+    private:
+        RawArray x_;
+};
+
+/*! \brief
+ * Casts a gmx::BasicVector array into an equivalent raw C array.
+ */
+template <typename ValueType> static inline
+typename BasicVector<ValueType>::RawArray *
+as_vec_array(BasicVector<ValueType> *x)
+{
+    return reinterpret_cast<typename BasicVector<ValueType>::RawArray *>(x);
+}
+
+/*! \brief
+ * Casts a gmx::BasicVector array into an equivalent raw C array.
+ */
+template <typename ValueType> static inline
+const typename BasicVector<ValueType>::RawArray *
+as_vec_array(const BasicVector<ValueType> *x)
+{
+    return reinterpret_cast<const typename BasicVector<ValueType>::RawArray *>(x);
+}
+
+//! Shorthand for C++ `rvec`-equivalent type.
+typedef BasicVector<real> RVec;
+//! Casts a gmx::RVec array into an `rvec` array.
+static inline rvec *as_rvec_array(RVec *x)
+{
+    return as_vec_array(x);
+}
+//! Casts a gmx::RVec array into an `rvec` array.
+static inline const rvec *as_rvec_array(const RVec *x)
+{
+    return as_vec_array(x);
+}
+
+} // namespace gmx
+
+#endif
+
 #endif
index e7821270358042de63c7939fb9ef251603e7004c..a8bd1addda128714a7da8ec9c851da8433200ed7 100644 (file)
@@ -70,7 +70,6 @@
 #include "gromacs/utility/arrayref.h"
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/gmxassert.h"
-#include "gromacs/utility/smalloc.h"
 #include "gromacs/utility/stringutil.h"
 
 namespace gmx
@@ -284,10 +283,8 @@ class AnalysisNeighborhoodSearchImpl
         bool                    bTric_;
         //! Whether the grid is periodic in a dimension.
         bool                    bGridPBC_[DIM];
-        //! Array allocated for storing in-unit-cell reference positions.
-        rvec                   *xref_alloc_;
-        //! Allocation count for xref_alloc.
-        int                     xref_nalloc_;
+        //! Array for storing in-unit-cell reference positions.
+        std::vector<RVec>       xrefAlloc_;
         //! Origin of the grid (zero for periodic dimensions).
         rvec                    gridOrigin_;
         //! Size of a single grid cell.
@@ -415,8 +412,6 @@ AnalysisNeighborhoodSearchImpl::AnalysisNeighborhoodSearchImpl(real cutoff)
     bGridPBC_[YY]   = true;
     bGridPBC_[ZZ]   = true;
 
-    xref_alloc_     = NULL;
-    xref_nalloc_    = 0;
     clear_rvec(gridOrigin_);
     clear_rvec(cellSize_);
     clear_rvec(invCellSize_);
@@ -431,7 +426,6 @@ AnalysisNeighborhoodSearchImpl::~AnalysisNeighborhoodSearchImpl()
         GMX_RELEASE_ASSERT(i->unique(),
                            "Dangling AnalysisNeighborhoodPairSearch reference");
     }
-    sfree(xref_alloc_);
 }
 
 AnalysisNeighborhoodSearchImpl::PairSearchImplPointer
@@ -924,17 +918,13 @@ void AnalysisNeighborhoodSearchImpl::init(
     }
     if (bGrid_)
     {
-        if (xref_nalloc_ < nref_)
-        {
-            srenew(xref_alloc_, nref_);
-            xref_nalloc_ = nref_;
-        }
-        xref_ = xref_alloc_;
+        xrefAlloc_.resize(nref_);
+        xref_ = as_rvec_array(&xrefAlloc_[0]);
 
         for (int i = 0; i < nref_; ++i)
         {
             rvec refcell;
-            mapPointToGridCell(positions.x_[i], refcell, xref_alloc_[i]);
+            mapPointToGridCell(positions.x_[i], refcell, xrefAlloc_[i]);
             addToGridCell(refcell, i);
         }
     }
index ed4f335d9df2ab59cb0d82b263417bd6ae2307ed..4419c2b27d07c6a151e52ed521b1d9b99835a358 100644 (file)
@@ -49,6 +49,8 @@
 #ifndef GMX_SELECTION_NBSEARCH_H
 #define GMX_SELECTION_NBSEARCH_H
 
+#include <vector>
+
 #include <boost/shared_ptr.hpp>
 
 #include "gromacs/math/vectypes.h"
@@ -118,6 +120,14 @@ class AnalysisNeighborhoodPositions
             : count_(count), index_(-1), x_(x), exclusionIds_(NULL)
         {
         }
+        /*! \brief
+         * Initializes positions from a vector of position vectors.
+         */
+        AnalysisNeighborhoodPositions(const std::vector<RVec> &x)
+            : count_(x.size()), index_(-1), x_(as_rvec_array(&x[0])),
+              exclusionIds_(NULL)
+        {
+        }
 
         /*! \brief
          * Sets indices to use for mapping exclusions to these positions.
index c38dd4b3fdaea6ee5aa3b7a80bb7257bf362fe36..5d876aac3e4018c46a7211f50c5731649bfbab75 100644 (file)
@@ -120,20 +120,19 @@ class NeighborhoodSearchTestData
 
         gmx::AnalysisNeighborhoodPositions refPositions() const
         {
-            return gmx::AnalysisNeighborhoodPositions(refPos_, refPosCount_);
+            return gmx::AnalysisNeighborhoodPositions(refPos_);
         }
         gmx::AnalysisNeighborhoodPositions testPositions() const
         {
-            if (testPos_ == NULL)
+            if (testPos_.empty())
             {
-                snew(testPos_, testPositions_.size());
+                testPos_.reserve(testPositions_.size());
                 for (size_t i = 0; i < testPositions_.size(); ++i)
                 {
-                    copy_rvec(testPositions_[i].x, testPos_[i]);
+                    testPos_.push_back(testPositions_[i].x);
                 }
             }
-            return gmx::AnalysisNeighborhoodPositions(testPos_,
-                                                      testPositions_.size());
+            return gmx::AnalysisNeighborhoodPositions(testPos_);
         }
         gmx::AnalysisNeighborhoodPositions testPosition(int index) const
         {
@@ -142,11 +141,11 @@ class NeighborhoodSearchTestData
 
         void addTestPosition(const rvec x)
         {
-            GMX_RELEASE_ASSERT(testPos_ == NULL,
+            GMX_RELEASE_ASSERT(testPos_.empty(),
                                "Cannot add positions after testPositions() call");
             testPositions_.push_back(TestPosition(x));
         }
-        void generateRandomPosition(rvec x);
+        gmx::RVec generateRandomPosition() const;
         void generateRandomRefPositions(int count);
         void generateRandomTestPositions(int count);
         void computeReferences(t_pbc *pbc)
@@ -175,20 +174,20 @@ class NeighborhoodSearchTestData
         matrix                           box_;
         t_pbc                            pbc_;
         int                              refPosCount_;
-        rvec                            *refPos_;
+        std::vector<gmx::RVec>           refPos_;
         TestPositionList                 testPositions_;
 
     private:
         void computeReferencesInternal(t_pbc *pbc, bool bXY);
 
-        mutable rvec                    *testPos_;
+        mutable std::vector<gmx::RVec>   testPos_;
 };
 
 //! Shorthand for a collection of reference pairs.
 typedef std::vector<NeighborhoodSearchTestData::RefPair> RefPairList;
 
 NeighborhoodSearchTestData::NeighborhoodSearchTestData(int seed, real cutoff)
-    : rng_(NULL), cutoff_(cutoff), refPosCount_(0), refPos_(NULL), testPos_(NULL)
+    : rng_(NULL), cutoff_(cutoff), refPosCount_(0)
 {
     // TODO: Handle errors.
     rng_ = gmx_rng_init(seed);
@@ -202,13 +201,11 @@ NeighborhoodSearchTestData::~NeighborhoodSearchTestData()
     {
         gmx_rng_destroy(rng_);
     }
-    sfree(refPos_);
-    sfree(testPos_);
 }
 
-void NeighborhoodSearchTestData::generateRandomPosition(rvec x)
+gmx::RVec NeighborhoodSearchTestData::generateRandomPosition() const
 {
-    rvec fx;
+    rvec fx, x;
     fx[XX] = gmx_rng_uniform_real(rng_);
     fx[YY] = gmx_rng_uniform_real(rng_);
     fx[ZZ] = gmx_rng_uniform_real(rng_);
@@ -217,15 +214,16 @@ void NeighborhoodSearchTestData::generateRandomPosition(rvec x)
     x[XX] += 0.2 * gmx_rng_uniform_real(rng_) - 0.1;
     x[YY] += 0.2 * gmx_rng_uniform_real(rng_) - 0.1;
     x[ZZ] += 0.2 * gmx_rng_uniform_real(rng_) - 0.1;
+    return x;
 }
 
 void NeighborhoodSearchTestData::generateRandomRefPositions(int count)
 {
     refPosCount_ = count;
-    snew(refPos_, refPosCount_);
-    for (int i = 0; i < refPosCount_; ++i)
+    refPos_.reserve(count);
+    for (int i = 0; i < count; ++i)
     {
-        generateRandomPosition(refPos_[i]);
+        refPos_.push_back(generateRandomPosition());
     }
 }
 
@@ -234,9 +232,7 @@ void NeighborhoodSearchTestData::generateRandomTestPositions(int count)
     testPositions_.reserve(count);
     for (int i = 0; i < count; ++i)
     {
-        rvec x;
-        generateRandomPosition(x);
-        addTestPosition(x);
+        addTestPosition(generateRandomPosition());
     }
 }
 
@@ -464,7 +460,8 @@ std::string formatVector(const rvec x)
 /*! \brief
  * Helper function to check that all expected pairs were found.
  */
-void checkAllPairsFound(const RefPairList &refPairs, const rvec refPos[],
+void checkAllPairsFound(const RefPairList &refPairs,
+                        const std::vector<gmx::RVec> &refPos,
                         int testPosIndex, const rvec testPos)
 {
     // This could be elegantly expressed with Google Mock matchers, but that
index e63cb502d644bf4cb953b9500bddde905be5570b..200aed1924fc44c206c63639f48936e4a74b3823 100644 (file)
@@ -244,7 +244,6 @@ class Angle : public TrajectoryAnalysisModule
 {
     public:
         Angle();
-        virtual ~Angle();
 
         virtual void initOptions(Options                    *options,
                                  TrajectoryAnalysisSettings *settings);
@@ -284,8 +283,7 @@ class Angle : public TrajectoryAnalysisModule
         std::vector<int>                         angleCount_;
         int                                      natoms1_;
         int                                      natoms2_;
-        // TODO: It is not possible to put rvec into a container.
-        std::vector<rvec *>                      vt0_;
+        std::vector<std::vector<RVec> >          vt0_;
 
         // Copy and assign disallowed by base.
 };
@@ -305,15 +303,6 @@ Angle::Angle()
 }
 
 
-Angle::~Angle()
-{
-    for (size_t g = 0; g < vt0_.size(); ++g)
-    {
-        delete [] vt0_[g];
-    }
-}
-
-
 void
 Angle::initOptions(Options *options, TrajectoryAnalysisSettings * /*settings*/)
 {
@@ -584,7 +573,7 @@ Angle::initAnalysis(const TrajectoryAnalysisSettings &settings,
         vt0_.resize(sel1_.size());
         for (size_t g = 0; g < sel1_.size(); ++g)
         {
-            vt0_[g] = new rvec[sel1_[g].posCount() / natoms1_];
+            vt0_[g].resize(sel1_[g].posCount() / natoms1_);
         }
     }
 
index 8ca70e1747c1435d822f1bed5dc882853fed8d17..0e0cf57627a17fe48d603cc8691adc58e60ad7ef 100644 (file)
@@ -69,7 +69,7 @@ class SurfaceAreaTest : public ::testing::Test
 {
     public:
         SurfaceAreaTest()
-            : rng_(NULL), allocated_(0), x_(NULL), area_(0.0), volume_(0.0),
+            : rng_(NULL), area_(0.0), volume_(0.0),
               atomArea_(NULL), dotCount_(0), dots_(NULL)
         {
             // TODO: Handle errors.
@@ -82,34 +82,18 @@ class SurfaceAreaTest : public ::testing::Test
             {
                 gmx_rng_destroy(rng_);
             }
-            sfree(x_);
             sfree(atomArea_);
             sfree(dots_);
         }
 
-        void reserveSpace(int count)
-        {
-            GMX_RELEASE_ASSERT(allocated_ == 0 && x_ == NULL,
-                               "Cannot allocate data more than once");
-            snew(x_, count);
-            radius_.reserve(count);
-            index_.reserve(count);
-            allocated_ = count;
-        }
-
         void addSphere(real x, real y, real z, real radius,
                        bool bAddToIndex = true)
         {
-            const int index = radius_.size();
-            GMX_RELEASE_ASSERT(index < allocated_,
-                               "Not enough space allocated for test data");
-            x_[index][XX] = x;
-            x_[index][YY] = y;
-            x_[index][ZZ] = z;
             if (bAddToIndex)
             {
-                index_.push_back(radius_.size());
+                index_.push_back(x_.size());
             }
+            x_.push_back(gmx::RVec(x, y, z));
             radius_.push_back(radius);
         }
 
@@ -136,7 +120,9 @@ class SurfaceAreaTest : public ::testing::Test
 
         void generateRandomPositions(int count)
         {
-            reserveSpace(count);
+            x_.reserve(count);
+            radius_.reserve(count);
+            index_.reserve(count);
             for (int i = 0; i < count; ++i)
             {
                 rvec x;
@@ -147,7 +133,7 @@ class SurfaceAreaTest : public ::testing::Test
         }
         void translatePoints(real x, real y, real z)
         {
-            for (size_t i = 0; i < radius_.size(); ++i)
+            for (size_t i = 0; i < x_.size(); ++i)
             {
                 x_[i][XX] += x;
                 x_[i][YY] += y;
@@ -172,7 +158,8 @@ class SurfaceAreaTest : public ::testing::Test
                     {
                         gmx::SurfaceAreaCalculator calculator;
                         calculator.setDotCount(ndots);
-                        calculator.calculate(x_, &radius_[0], bPBC ? &pbc : NULL,
+                        calculator.calculate(as_rvec_array(&x_[0]), &radius_[0],
+                                             bPBC ? &pbc : NULL,
                                              index_.size(), &index_[0], flags,
                                              &area_, &volume_, &atomArea_,
                                              &dots_, &dotCount_);
@@ -239,24 +226,22 @@ class SurfaceAreaTest : public ::testing::Test
             return 0;
         }
 
-        gmx_rng_t          rng_;
-        int                allocated_;
-        rvec              *x_;
-        std::vector<real>  radius_;
-        std::vector<int>   index_;
-
-        real               area_;
-        real               volume_;
-        real              *atomArea_;
-        int                dotCount_;
-        real              *dots_;
+        gmx_rng_t               rng_;
+        std::vector<gmx::RVec>  x_;
+        std::vector<real>       radius_;
+        std::vector<int>        index_;
+
+        real                    area_;
+        real                    volume_;
+        real                   *atomArea_;
+        int                     dotCount_;
+        real                   *dots_;
 };
 
 TEST_F(SurfaceAreaTest, ComputesSinglePoint)
 {
     gmx::test::FloatingPointTolerance tolerance(
             gmx::test::defaultRealTolerance());
-    reserveSpace(1);
     addSphere(1, 1, 1, 1);
     ASSERT_NO_FATAL_FAILURE(calculate(24, FLAG_VOLUME | FLAG_ATOM_AREA, false));
     EXPECT_REAL_EQ_TOL(4*M_PI, resultArea(), tolerance);
@@ -268,7 +253,6 @@ TEST_F(SurfaceAreaTest, ComputesTwoPoints)
 {
     gmx::test::FloatingPointTolerance tolerance(
             gmx::test::relativeToleranceAsFloatingPoint(1.0, 0.005));
-    reserveSpace(2);
     addSphere(1, 1, 1, 1);
     addSphere(2, 1, 1, 1);
     ASSERT_NO_FATAL_FAILURE(calculate(1000, FLAG_ATOM_AREA, false));
@@ -281,7 +265,6 @@ TEST_F(SurfaceAreaTest, ComputesTwoPointsOfUnequalRadius)
 {
     gmx::test::FloatingPointTolerance tolerance(
             gmx::test::relativeToleranceAsFloatingPoint(1.0, 0.005));
-    reserveSpace(2);
     // Spheres of radius 1 and 2 with intersection at 1.5
     const real dist = 0.5 + sqrt(3.25);
     addSphere(1.0, 1.0, 1.0, 1);
@@ -295,7 +278,6 @@ TEST_F(SurfaceAreaTest, ComputesTwoPointsOfUnequalRadius)
 TEST_F(SurfaceAreaTest, SurfacePoints12)
 {
     gmx::test::TestReferenceChecker checker(data_.rootChecker());
-    reserveSpace(1);
     addSphere(0, 0, 0, 1);
     ASSERT_NO_FATAL_FAILURE(calculate(12, FLAG_DOTS, false));
     checkReference(&checker, "Surface", true);
@@ -304,7 +286,6 @@ TEST_F(SurfaceAreaTest, SurfacePoints12)
 TEST_F(SurfaceAreaTest, SurfacePoints32)
 {
     gmx::test::TestReferenceChecker checker(data_.rootChecker());
-    reserveSpace(1);
     addSphere(0, 0, 0, 1);
     ASSERT_NO_FATAL_FAILURE(calculate(32, FLAG_DOTS, false));
     checkReference(&checker, "Surface", true);
@@ -313,7 +294,6 @@ TEST_F(SurfaceAreaTest, SurfacePoints32)
 TEST_F(SurfaceAreaTest, SurfacePoints42)
 {
     gmx::test::TestReferenceChecker checker(data_.rootChecker());
-    reserveSpace(1);
     addSphere(0, 0, 0, 1);
     ASSERT_NO_FATAL_FAILURE(calculate(42, FLAG_DOTS, false));
     checkReference(&checker, "Surface", true);
@@ -322,7 +302,6 @@ TEST_F(SurfaceAreaTest, SurfacePoints42)
 TEST_F(SurfaceAreaTest, SurfacePoints122)
 {
     gmx::test::TestReferenceChecker checker(data_.rootChecker());
-    reserveSpace(1);
     addSphere(0, 0, 0, 1);
     ASSERT_NO_FATAL_FAILURE(calculate(122, FLAG_DOTS, false));
     checkReference(&checker, "Surface", true);