Improve AbstractAnalysisData data access interface.
authorTeemu Murtola <teemu.murtola@gmail.com>
Tue, 27 Dec 2011 20:15:25 +0000 (22:15 +0200)
committerTeemu Murtola <teemu.murtola@gmail.com>
Wed, 22 Feb 2012 05:38:08 +0000 (07:38 +0200)
Access to stored data is now done through wrapper object instead of
retrieving raw pointers.  Also refactored the implementation such that
the pure virtual methods that implement the actual data access are now
protected, and the public members are now simple non-virtual wrappers
that perform common checking and indexing.

Also removed support for negative data frame indexing.  It is no longer
necessary because the data frame index is now passed to
AnalysisDataModuleInterface::pointsAdded(), so it's easy to compute the
absolute index of past frames, and additionally it didn't work properly
in parallelization schemes.  Also removed possibility to access the
current frame from pointsAdded(), as it would just create complications
to allow access to an incomplete frame.

Part of issue #827.

Change-Id: If932c3f6b8394f8ceacc632fb9d87ec1933f56ca

16 files changed:
src/gromacs/analysisdata/abstractdata-impl.h
src/gromacs/analysisdata/abstractdata.cpp
src/gromacs/analysisdata/abstractdata.h
src/gromacs/analysisdata/analysisdata.cpp
src/gromacs/analysisdata/arraydata.cpp
src/gromacs/analysisdata/arraydata.h
src/gromacs/analysisdata/dataframe.cpp
src/gromacs/analysisdata/dataframe.h
src/gromacs/analysisdata/dataproxy.cpp
src/gromacs/analysisdata/dataproxy.h
src/gromacs/analysisdata/modules/displacement.cpp
src/gromacs/analysisdata/modules/displacement.h
src/gromacs/analysisdata/tests/analysisdata.cpp
src/gromacs/analysisdata/tests/datatest.h
src/gromacs/analysisdata/tests/mock_module.cpp
src/gromacs/trajectoryanalysis/modules/distance.cpp

index 9b754daba00058c10d4f5836b0ec11e9df4b420e..939e3469504fec11247797a595affd4b11f3e78d 100644 (file)
@@ -68,12 +68,15 @@ class AbstractAnalysisData::Impl
          *
          * \param[in] data   Data object to read data from.
          * \param[in] module Module to present the data to.
-         * \retval ::eeInvalidValue if \p module is not compatible with the
+         * \exception APIError if \p module is not compatible with the
          *      data object.
-         * \retval ::eedataDataNotAvailable if all data is not  available
-         *      through getData().
+         * \exception APIError if all data is not  available
+         *      through getDataFrame().
          *
-         * Uses getData() in \p data to access all data in the object, and
+         * Other exceptions may also be thrown if methods called in \p module
+         * throw them.
+         *
+         * Uses getDataFrame() in \p data to access all data in the object, and
          * calls the notification functions in \p module as if the module had
          * been registered to the data object when the data was added.
          */
@@ -95,7 +98,7 @@ class AbstractAnalysisData::Impl
         /*! \brief
          * Total number of frames in the data.
          *
-         * The counter is incremented in notifyFrameStart().
+         * The counter is incremented in notifyFrameFinish().
          */
         int                     _nframes;
 };
@@ -108,18 +111,17 @@ class AbstractAnalysisData::Impl
 class AnalysisDataFrame
 {
     public:
-        AnalysisDataFrame();
+        explicit AnalysisDataFrame(int columnCount);
         ~AnalysisDataFrame();
 
-        //! Allocate memory for a given number of columns.
-        void allocate(int ncol);
-
         //! Zero-based global index of the frame.
         int                     _index;
         //! x value of the frame.
         real                    _x;
         //! Error of x for the frame.
         real                    _dx;
+        //! Number of columns in the frame.
+        int                     _columnCount;
         //! Array of column values for the frame.
         real                   *_y;
         //! Array of column error values for the frame.
index dda3d19ebea23a373b4ded550ea75c6c841c3126..249f58e2637e41a5b3ee8e1b8f748b0a2a90cc82 100644 (file)
@@ -82,29 +82,15 @@ AbstractAnalysisData::Impl::presentData(AbstractAnalysisData *data,
     int ncol = data->columnCount();
     for (int i = 0; i < data->frameCount(); ++i)
     {
-        real        x, dx;
-        const real *y, *dy;
-        const bool *present;
-
-        if (!data->getDataWErr(i, &x, &dx, &y, &dy, &present))
-        {
-            GMX_THROW(APIError("Data not available when module added"));
-        }
-        if (bCheckMissing && present)
+        AnalysisDataFrameRef frame = data->getDataFrame(i);
+        GMX_RELEASE_ASSERT(frame.isValid(), "Invalid data frame returned");
+        if (bCheckMissing && !frame.allPresent())
         {
-            for (int j = 0; j < ncol; ++j)
-            {
-                if (!present[j])
-                {
-                    GMX_THROW(APIError("Missing data not supported by a module"));
-                }
-            }
+            GMX_THROW(APIError("Missing data not supported by a module"));
         }
-        AnalysisDataFrameHeader header(i, x, dx);
-        module->frameStarted(header);
-        module->pointsAdded(
-                AnalysisDataPointSetRef(header, 0, ncol, y, dy, present));
-        module->frameFinished(header);
+        module->frameStarted(frame.header());
+        module->pointsAdded(frame.points());
+        module->frameFinished(frame.header());
     }
     if (!_bInData)
     {
@@ -136,18 +122,38 @@ AbstractAnalysisData::frameCount() const
 }
 
 
-bool
-AbstractAnalysisData::getData(int index, real *x, const real **y,
-                              const bool **missing) const
+AnalysisDataFrameRef
+AbstractAnalysisData::tryGetDataFrame(int index) const
 {
-    return getDataWErr(index, x, 0, y, 0, missing);
+    if (index < 0 || index >= frameCount())
+    {
+        return AnalysisDataFrameRef();
+    }
+    return tryGetDataFrameInternal(index);
+}
+
+
+AnalysisDataFrameRef
+AbstractAnalysisData::getDataFrame(int index) const
+{
+    AnalysisDataFrameRef frame = tryGetDataFrame(index);
+    if (!frame.isValid())
+    {
+        GMX_THROW(APIError("Invalid frame accessed"));
+    }
+    return frame;
 }
 
 
 bool
-AbstractAnalysisData::getErrors(int index, real *dx, const real **dy) const
+AbstractAnalysisData::requestStorage(int nframes)
 {
-    return getDataWErr(index, 0, dx, 0, dy, 0);
+    GMX_RELEASE_ASSERT(nframes >= -1, "Invalid number of frames requested");
+    if (nframes == 0)
+    {
+        return true;
+    }
+    return requestStorageInternal(nframes);
 }
 
 
@@ -247,7 +253,6 @@ AbstractAnalysisData::notifyDataStart()
     _impl->_bDataStart = _impl->_bInData = true;
 
     Impl::ModuleList::const_iterator i;
-
     for (i = _impl->_modules.begin(); i != _impl->_modules.end(); ++i)
     {
         if (_ncol > 1 && !((*i)->flags() & AnalysisDataModuleInterface::efAllowMulticolumn))
@@ -269,7 +274,6 @@ AbstractAnalysisData::notifyFrameStart(const AnalysisDataFrameHeader &header) co
                "Out of order frames");
     _impl->_bInFrame = true;
     _impl->_currHeader = header;
-    ++_impl->_nframes;
 
     Impl::ModuleList::const_iterator i;
     for (i = _impl->_modules.begin(); i != _impl->_modules.end(); ++i)
@@ -319,8 +323,11 @@ AbstractAnalysisData::notifyFrameFinish(const AnalysisDataFrameHeader &header) c
                "Header does not correspond to current frame");
     _impl->_bInFrame = false;
 
-    Impl::ModuleList::const_iterator i;
+    // Increment the counter before notifications to allow frame access from
+    // modules.
+    ++_impl->_nframes;
 
+    Impl::ModuleList::const_iterator i;
     for (i = _impl->_modules.begin(); i != _impl->_modules.end(); ++i)
     {
         (*i)->frameFinished(header);
@@ -344,7 +351,6 @@ AbstractAnalysisData::notifyDataFinish() const
     _impl->_bInData = false;
 
     Impl::ModuleList::const_iterator i;
-
     for (i = _impl->_modules.begin(); i != _impl->_modules.end(); ++i)
     {
         (*i)->dataFinished();
@@ -356,11 +362,16 @@ AbstractAnalysisData::notifyDataFinish() const
  * AnalysisDataFrame
  */
 
-AnalysisDataFrame::AnalysisDataFrame()
-    : _y(NULL), _dy(NULL), _present(NULL)
+AnalysisDataFrame::AnalysisDataFrame(int columnCount)
+    : _index(-1), _x(0.0), _dx(0.0), _columnCount(columnCount),
+      _y(NULL), _dy(NULL), _present(NULL)
 {
+    snew(_y, columnCount);
+    snew(_dy, columnCount);
+    snew(_present, columnCount);
 }
 
+
 AnalysisDataFrame::~AnalysisDataFrame()
 {
     sfree(_y);
@@ -369,14 +380,6 @@ AnalysisDataFrame::~AnalysisDataFrame()
 }
 
 
-void AnalysisDataFrame::allocate(int ncol)
-{
-    snew(_y, ncol);
-    snew(_dy, ncol);
-    snew(_present, ncol);
-}
-
-
 /********************************************************************
  * AbstractAnalysisDataStored::Impl
  */
@@ -439,51 +442,22 @@ AbstractAnalysisDataStored::~AbstractAnalysisDataStored()
 }
 
 
-bool
-AbstractAnalysisDataStored::getDataWErr(int index, real *x, real *dx,
-                                        const real **y, const real **dy,
-                                        const bool **present) const
+AnalysisDataFrameRef
+AbstractAnalysisDataStored::tryGetDataFrameInternal(int index) const
 {
-    index = _impl->getStoreIndex(index, frameCount());
-    if (index < 0)
-    {
-        return false;
-    }
-
-    // Retrieve the data.
-    AnalysisDataFrame *fr = _impl->_store[index];
-    if (x)
-    {
-        *x = fr->_x;
-    }
-    if (dx)
-    {
-        *dx = fr->_dx;
-    }
-    if (y)
+    if (!_impl->_bStoreAll && index < frameCount() - _impl->_nalloc)
     {
-        *y = fr->_y;
+        return AnalysisDataFrameRef();
     }
-    if (dy)
-    {
-        *dy = fr->_dy;
-    }
-    if (present)
-    {
-        *present = fr->_present;
-    }
-    return true;
+    AnalysisDataFrame *fr = _impl->_store[index % _impl->_nalloc];
+    return AnalysisDataFrameRef(fr->_index, fr->_x, fr->_dx, fr->_columnCount,
+                                fr->_y, fr->_dy, fr->_present);
 }
 
 
 bool
-AbstractAnalysisDataStored::requestStorage(int nframes)
+AbstractAnalysisDataStored::requestStorageInternal(int nframes)
 {
-    GMX_RELEASE_ASSERT(nframes >= -1, "Invalid number of frames requested");
-    if (nframes == 0)
-    {
-        return true;
-    }
     GMX_RELEASE_ASSERT(!isMultipoint(), "Storage of multipoint data not supported");
 
     // Handle the case when everything needs to be stored.
@@ -520,16 +494,13 @@ AbstractAnalysisDataStored::startDataStore()
     // some storage.
     notifyDataStart();
 
-    int ncol = columnCount();
-
     // If any storage has been requested, preallocate it.
     if (_impl->_nalloc > 0)
     {
         _impl->_store.resize(_impl->_nalloc);
         for (int i = 0; i < _impl->_nalloc; ++i)
         {
-            _impl->_store[i] = new AnalysisDataFrame();
-            _impl->_store[i]->allocate(ncol);
+            _impl->_store[i] = new AnalysisDataFrame(columnCount());
         }
         _impl->_nextind = 0;
     }
@@ -546,14 +517,11 @@ AbstractAnalysisDataStored::startNextFrame(const AnalysisDataFrameHeader &header
         {
             if (_impl->_bStoreAll)
             {
-                int ncol = columnCount();
-
                 _impl->_nalloc = _impl->_nextind + 1;
                 _impl->_store.resize(_impl->_nalloc);
                 for (int i = _impl->_nextind; i < _impl->_nalloc; ++i)
                 {
-                    _impl->_store[i] = new AnalysisDataFrame();
-                    _impl->_store[i]->allocate(ncol);
+                    _impl->_store[i] = new AnalysisDataFrame(columnCount());
                 }
             }
             else
@@ -600,7 +568,7 @@ AbstractAnalysisDataStored::storeThisFrame(const real *y, const real *dy,
     // Notify modules of new data.
     notifyPointsAdd(0, ncol, y, dy, present);
     // The index needs to be incremented after the notifications to allow
-    // the modules to use getData() properly.
+    // the modules to use getDataFrame() properly.
     if (_impl->_nextind >= 0)
     {
         ++_impl->_nextind;
index 21430226943d9f05cc626e5f9103f0fa4fef14fb..34be73652e5c7d84972cec4aae2674fb8db28bea 100644 (file)
@@ -46,14 +46,15 @@ namespace gmx
 
 class AnalysisDataModuleInterface;
 class AnalysisDataFrameHeader;
+class AnalysisDataFrameRef;
 class AnalysisDataPointSetRef;
 
 /*! \brief
  * Abstract base class for all objects that provide data.
  *
  * The public interface includes functions for querying the data
- * (isMultipoint(), columnCount(), frameCount(), getDataWErr(), getData(),
- * getErrors(), requestStorage()) and functions for using modules for
+ * (isMultipoint(), columnCount(), frameCount(), tryGetDataFrame(),
+ * getDataFrame(), requestStorage()) and functions for using modules for
  * processing the data (addModule(), addColumnModule(), applyModule()).
  *
  * \if libapi
@@ -131,63 +132,53 @@ class AbstractAnalysisData
          *
          * This function returns the number of frames that the object has
          * produced. If requestStorage() has been successfully called,
-         * getData() can be used to access some or all of these frames.
+         * getDataFrame() can be used to access some or all of these frames.
          */
         int frameCount() const;
         /*! \brief
          * Access stored data.
          *
-         * \param[in]  index   Frame index to access
-         *      (negative indices count backwards from the current frame).
-         * \param[out] x
-         * \param[out] dx
-         * \param[out] y
-         * \param[out] dy
-         * \param[out] present Returns a pointer to an array that tells
-         *      whether the corresponding column is present in that frame.
-         *      If NULL, no missing information is returned.
-         * \retval \c false if data for the requested frame is no longer
-         *      available.
+         * \param[in] index  Zero-based frame index to access.
+         * \retval    Frame reference to frame \p index, or an invalid
+         *      reference if no such frame is available.
          *
-         * \if libapi
-         * Derived classes can choose to return false if requestStorage() has
-         * not been called at all, or if the frame is too old (compared to the
-         * value given to requestStorage()).
-         * \endif
+         * Does not throw.  Failure to access a frame with the given index is
+         * indicated through the return value.  Negative \p index is allowed,
+         * and will always result in an invalid reference being returned.
          *
-         * \todo
-         * For more flexibility, it would be better to return a data row/frame
-         * object from this method, which could then be used to access all the
-         * data for that frame.
+         * \see requestStorage()
+         * \see getDataFrame()
          */
-        virtual bool getDataWErr(int index, real *x, real *dx,
-                                 const real **y, const real **dy,
-                                 const bool **present = 0) const = 0;
+        AnalysisDataFrameRef tryGetDataFrame(int index) const;
         /*! \brief
-         * Convenience function for accessing stored data.
+         * Access stored data.
          *
-         * \see getDataWErr()
-         */
-        bool getData(int index, real *x, const real **y,
-                     const bool **present = 0) const;
-        /*! \brief
-         * Convenience function for accessing errors for stored data.
+         * \param[in] index  Zero-based frame index to access.
+         * \retval    Frame reference to frame \p index.
+         * \exception APIError if the requested frame is accessible.
+         *
+         * If the data is not certainly available, use tryGetDataFrame().
          *
-         * \see getDataWErr()
+         * \see requestStorage()
+         * \see tryGetDataFrame()
          */
-        bool getErrors(int index, real *dx, const real **dy) const;
+        AnalysisDataFrameRef getDataFrame(int index) const;
         /*! \brief
          * Request storage of frames.
          *
          * \param[in] nframes  Request storing at least \c nframes previous
-         *     frames (-1 = request storing all).
+         *     frames (-1 = request storing all). Must be >= -1.
          * \retval true if the request could be satisfied.
          *
-         * If called multiple times, the largest request should be honored.
+         * If called multiple times, the largest request is honored.
          *
-         * \see getData()
+         * Does not throw.  Failure to honor the request is indicated through
+         * the return value.
+         *
+         * \see getDataFrame()
+         * \see tryGetDataFrame()
          */
-        virtual bool requestStorage(int nframes = -1) = 0;
+        bool requestStorage(int nframes);
 
         /*! \brief
          * Adds a module to process the data.
@@ -196,11 +187,11 @@ class AbstractAnalysisData
          * \exception APIError if
          *      - \p module is not compatible with the data object
          *      - data has already been added to the data object and everything
-         *        is not available through getData().
+         *        is not available through getDataFrame().
          *
          * If data has already been added to the module, the new module
          * immediately processes all existing data.  APIError is thrown
-         * if all data is not available through getData().
+         * if all data is not available through getDataFrame().
          *
          * When this function is entered, the data object takes ownership of the
          * module, and automatically destructs it when the data object itself
@@ -228,7 +219,7 @@ class AbstractAnalysisData
          *
          * This function works as addModule(), except that it does not take
          * ownership of \p module. Also, it can only be called after the data
-         * is ready, and only if getData() gives access to all of the data.
+         * is ready, and only if getDataFrame() gives access to all of the data.
          * It is provided for additional flexibility in postprocessing
          * in-memory data.
          */
@@ -264,6 +255,44 @@ class AbstractAnalysisData
          */
         void setMultipoint(bool multipoint);
 
+        /*! \brief
+         * Implements access to data frames.
+         *
+         * \param[in] index  Zero-based frame index to access.
+         * \retval    Frame reference to frame \p index, or an invalid
+         *      reference if no such frame is available.
+         *
+         * Must not throw.  Failure to access a frame with the given index is
+         * indicated through the return value.
+         *
+         * Code in derived classes can assume that \p index is non-negative and
+         * less than frameCount().
+         *
+         * Derived classes can choose to return an invalid reference if
+         * requestStorageInternal() has not been called at all, or if the frame
+         * is too old (compared to the value given to requestStorageInternal()).
+         *
+         * This method is called internally by tryGetDataFrame() and
+         * getDataFrame().
+         */
+        virtual AnalysisDataFrameRef tryGetDataFrameInternal(int index) const = 0;
+        /*! \brief
+         * Implements storage requests.
+         *
+         * \param[in] nframes  Request storing at least \c nframes previous
+         *     frames (-1 = request storing all). Will be either -1 or >0.
+         * \retval true if the request could be satisfied.
+         *
+         * Must not throw.  Failure to access a frame with the given index is
+         * indicated through the return value.
+         *
+         * Derived classes should be prepared for any number of calls to this
+         * method before notifyDataStart() is called (and during that call).
+         *
+         * This method is called internally by requestStorage().
+         */
+        virtual bool requestStorageInternal(int nframes) = 0;
+
         /*! \brief
          * Notifies attached modules of the start of data.
          *
@@ -363,11 +392,6 @@ class AbstractAnalysisDataStored : public AbstractAnalysisData
     public:
         virtual ~AbstractAnalysisDataStored();
 
-        virtual bool getDataWErr(int index, real *x, real *dx,
-                                 const real **y, const real **dy,
-                                 const bool **present = 0) const;
-        virtual bool requestStorage(int nframes = -1);
-
     protected:
         /*! \cond libapi */
         AbstractAnalysisDataStored();
@@ -391,6 +415,9 @@ class AbstractAnalysisDataStored : public AbstractAnalysisData
         //! \endcond
 
     private:
+        virtual AnalysisDataFrameRef tryGetDataFrameInternal(int index) const;
+        virtual bool requestStorageInternal(int nframes);
+
         class Impl;
 
         Impl                   *_impl;
index b7f18d4c2751f5172ab830af8a0bd1bbd8c5ec88..0734bb48787f8e91d02721bf6507d1736eac4f59 100644 (file)
@@ -198,8 +198,7 @@ AnalysisData::startData(AnalysisDataParallelOptions opt)
     Impl::FrameList::iterator i;
     for (i = _impl->_pending.begin() + oldSize; i != _impl->_pending.end(); ++i)
     {
-        *i = new AnalysisDataFrame();
-        (*i)->allocate(columnCount());
+        *i = new AnalysisDataFrame(columnCount());
         (*i)->_index = -1;
     }
 
@@ -235,8 +234,7 @@ AnalysisDataHandle::Impl::Impl(AnalysisData *data)
 {
     if (!_data.isMultipoint())
     {
-        _frame.reset(new AnalysisDataFrame());
-        _frame->allocate(_data.columnCount());
+        _frame.reset(new AnalysisDataFrame(_data.columnCount()));
     }
 }
 
index ca64aa8c21904f5b5f620edf2a2ece2f0a6f9bed..94e90bec3c0d4fced7ca822761c7511b6b6c0d2e 100644 (file)
@@ -56,51 +56,16 @@ AbstractAnalysisArrayData::~AbstractAnalysisArrayData()
 }
 
 
-bool
-AbstractAnalysisArrayData::getDataWErr(int index, real *x, real *dx,
-                                       const real **y, const real **dy,
-                                       const bool **present) const
+AnalysisDataFrameRef
+AbstractAnalysisArrayData::tryGetDataFrameInternal(int index) const
 {
-    if (index < 0)
-    {
-        index += frameCount() - 1;
-        if (index < 0)
-        {
-            return false;
-        }
-    }
-    if (index >= frameCount())
-    {
-        return false;
-    }
-    if (x != NULL)
-    {
-        *x = xvalue(index);
-    }
-    if (dx != NULL)
-    {
-        *dx = 0.0;
-    }
-    if (y != NULL)
-    {
-        *y = &_value[index * columnCount()];
-    }
-    if (dy != NULL)
-    {
-        // TODO: Implement
-        *dy = NULL;
-    }
-    if (present != NULL)
-    {
-        // TODO: Implement
-        *present = NULL;
-    }
-    return true;
+    return AnalysisDataFrameRef(index, xvalue(index), 0.0, columnCount(),
+                                &_value[index * columnCount()], NULL, NULL);
 }
 
 
 bool
-AbstractAnalysisArrayData::requestStorage(int /*nframes*/)
+AbstractAnalysisArrayData::requestStorageInternal(int /*nframes*/)
 {
     return true;
 }
index 4712a2fdac591d8a859b28a0dfad7021214ce0c7..1dc004f6f263bae46043be83124c9b942a223d1a 100644 (file)
@@ -66,11 +66,6 @@ class AbstractAnalysisArrayData : public AbstractAnalysisData
     public:
         virtual ~AbstractAnalysisArrayData();
 
-        virtual bool getDataWErr(int index, real *x, real *dx,
-                                 const real **y, const real **dy,
-                                 const bool **present = 0) const;
-        virtual bool requestStorage(int nframes = -1);
-
         /*! \brief
          * Returns the number of rows in the data array.
          *
@@ -160,6 +155,9 @@ class AbstractAnalysisArrayData : public AbstractAnalysisData
                                  AbstractAnalysisArrayData *dest);
 
     private:
+        virtual AnalysisDataFrameRef tryGetDataFrameInternal(int index) const;
+        virtual bool requestStorageInternal(int nframes);
+
         int                  _nrows;
         std::vector<real>    _value;
         real                 _xstart;
index 0c8ec1b5875305842c850c5e90ffb012abbe61a0..f31f31f271fb5bc2148ce5fcee8cea76468ff6e7 100644 (file)
@@ -146,4 +146,37 @@ bool AnalysisDataPointSetRef::allPresent() const
     return true;
 }
 
+
+/********************************************************************
+ * AnalysisDataFrameRef
+ */
+
+AnalysisDataFrameRef::AnalysisDataFrameRef()
+    : points_(AnalysisDataFrameHeader(), 0, 0, NULL, NULL, NULL)
+{
+}
+
+
+AnalysisDataFrameRef::AnalysisDataFrameRef(
+        int index, real x, real dx, int columnCount,
+        const real *y, const real *dy, const bool *present)
+    : points_(index, x, dx, 0, columnCount, y, dy, present)
+{
+}
+
+
+AnalysisDataFrameRef::AnalysisDataFrameRef(
+        const AnalysisDataFrameHeader &header, int columnCount,
+        const real *y, const real *dy, const bool *present)
+    : points_(header, 0, columnCount, y, dy, present)
+{
+}
+
+
+AnalysisDataFrameRef::AnalysisDataFrameRef(
+        const AnalysisDataFrameRef &frame, int firstColumn, int columnCount)
+    : points_(frame.points(), firstColumn, columnCount)
+{
+}
+
 } // namespace gmx
index bbff5999441a86d5a95a9bd0a7e0488b37824034..15fb60a6494204a248febdb133cc3060d92ae763 100644 (file)
@@ -143,6 +143,12 @@ class AnalysisDataFrameHeader
  * Methods in this class do not throw, but may contain asserts for incorrect
  * usage.
  *
+ * The design of the interfaces is such that all objects of this type should be
+ * valid, i.e., header().isValid() should always return true.  This is
+ * currently not strictly enforced in the constructors because of an
+ * implementation detail of AnalysisDataFrameRef, but this is subject to
+ * change.
+ *
  * \inpublicapi
  * \ingroup module_analysisdata
  */
@@ -203,7 +209,7 @@ class AnalysisDataPointSetRef
          * \param[in] columnCount Number of columns to include.
          *
          * Creates a point set that contains \p columnCount columns starting
-         * from \p firstColumn from \p points, or a subset all requested
+         * from \p firstColumn from \p points, or a subset if all requested
          * columns are not present in \p points.  If the requested column range
          * and the range in \p points do not intersect, the result has
          * columnCount() == 0.
@@ -309,6 +315,181 @@ class AnalysisDataPointSetRef
         const bool             *present_;
 };
 
+
+/*! \brief
+ * Value type wrapper for non-mutable access to a data frame.
+ *
+ * Default copy constructor and assignment operator are used and work as
+ * intended.
+ * Typically new objects of this type are only constructed internally by the
+ * library and in classes that are derived from AbstractAnalysisData.
+ *
+ * Methods in this class do not throw, but may contain asserts for incorrect
+ * usage.
+ *
+ * \todo
+ * Support for multipoint data.
+ *
+ * \inpublicapi
+ * \ingroup module_analysisdata
+ */
+class AnalysisDataFrameRef
+{
+    public:
+        /*! \brief
+         * Constructs an invalid frame reference.
+         *
+         * Return values of other methods than isValid() are unspecified for
+         * the constructed object.
+         */
+        AnalysisDataFrameRef();
+        /*! \brief
+         * Constructs a frame reference from given values.
+         *
+         * \param[in] index       Index of the frame. Must be >= 0.
+         * \param[in] x           x coordinate for the frame.
+         * \param[in] dx          Error estimate for x.
+         * \param[in] columnCount Number of columns to include. Must be >= 0.
+         * \param[in] y           Array of values for each column.
+         *     Must not be NULL if columnCount > 0.
+         * \param[in] dy          Array of error estimates for corresponding y.
+         *     Can be NULL, in which case errors cannot be accessed.
+         * \param[in] present     Array of flags giving presence of each point.
+         *     Can be NULL, in which case all values are treated as present.
+         *
+         * Arrays \p y, \p dy and \p dy should all have \p columnCount
+         * elements.
+         */
+        AnalysisDataFrameRef(int index, real x, real dx,
+                             int columnCount,
+                             const real *y, const real *dy,
+                             const bool *present);
+        /*! \brief
+         * Constructs a frame reference from given values.
+         *
+         * \param[in] header      Header for the frame.
+         * \param[in] columnCount Number of columns to include.
+         * \param[in] y           Array of values for each column.
+         *     Must not be NULL if columnCount > 0.
+         * \param[in] dy          Array of error estimates for corresponding y.
+         *     Can be NULL, in which case errors cannot be accessed.
+         * \param[in] present     Array of flags giving presence of each point.
+         *     Can be NULL, in which case all values are treated as present.
+         *
+         * Arrays \p y, \p dy and \p dy should all have \p columnCount
+         * elements.
+         */
+        AnalysisDataFrameRef(const AnalysisDataFrameHeader &header,
+                             int columnCount,
+                             const real *y, const real *dy,
+                             const bool *present);
+        /*! \brief
+         * Constructs a frame reference to a subset of columns.
+         *
+         * \param[in] frame       Frame to use as source.
+         * \param[in] firstColumn First column index to include.
+         * \param[in] columnCount Number of columns to include.
+         *
+         * Creates a frame reference that contains \p columnCount columns
+         * starting from \p firstColumn from \p frame, or a subset if all
+         * requested columns are not present in \p frame.
+         *
+         * Mainly intended for internal use.
+         */
+        AnalysisDataFrameRef(const AnalysisDataFrameRef &frame,
+                             int firstColumn, int columnCount);
+
+        /*! \brief
+         * Returns whether the object refers to a valid frame.
+         *
+         * If returns false, return values of other methods are not specified.
+         */
+        bool isValid() const
+        {
+            return header().isValid();
+        }
+        //! Returns the header for this frame.
+        const AnalysisDataFrameHeader &header() const
+        {
+            return points_.header();
+        }
+        //! \copydoc AnalysisDataFrameHeader::index()
+        int frameIndex() const
+        {
+            return header().index();
+        }
+        //! \copydoc AnalysisDataFrameHeader::x()
+        real x() const
+        {
+            return header().x();
+        }
+        //! \copydoc AnalysisDataFrameHeader::dx()
+        real dx() const
+        {
+            return header().dx();
+        }
+        /*! \brief
+         * Returns point set reference to the column values of this frame.
+         *
+         * Should not be called for invalid frames.
+         */
+        const AnalysisDataPointSetRef &points() const
+        {
+            GMX_ASSERT(isValid(), "Invalid data frame accessed");
+            return points_;
+        }
+        /*! \brief
+         * Returns number of columns in this frame.
+         *
+         * Returns zero for an invalid frame.
+         */
+        int columnCount() const
+        {
+            return points().columnCount();
+        }
+        /*! \brief
+         * Convenience method for accessing a column value.
+         *
+         * \copydetails AnalysisDataPointSetRef::y()
+         */
+        real y(int i) const
+        {
+            GMX_ASSERT(isValid(), "Invalid data frame accessed");
+            return points().y(i);
+        }
+        /*! \brief
+         * Convenience method for accessing error for a column value.
+         *
+         * \copydetails AnalysisDataPointSetRef::dy()
+         */
+        real dy(int i) const
+        {
+            GMX_ASSERT(isValid(), "Invalid data frame accessed");
+            return points().dy(i);
+        }
+        /*! \brief
+         * Convenience method for accessing present status for a column.
+         *
+         * \copydetails AnalysisDataPointSetRef::present()
+         */
+        bool present(int i) const
+        {
+            GMX_ASSERT(isValid(), "Invalid data frame accessed");
+            return points().present(i);
+        }
+        /*! \brief
+         * Returns true if all points in this frame are present.
+         */
+        bool allPresent() const
+        {
+            GMX_ASSERT(isValid(), "Invalid data frame accessed");
+            return points().allPresent();
+        }
+
+    private:
+        AnalysisDataPointSetRef points_;
+};
+
 } // namespace gmx
 
 #endif
index 8049ecd7cd90c89b1d108724b822c457fbcca894..b7ed9b53331af1621e7070615f5804eafda20c17 100644 (file)
@@ -54,40 +54,20 @@ AnalysisDataProxy::AnalysisDataProxy(int col, int span,
 }
 
 
-int
-AnalysisDataProxy::frameCount() const
-{
-    return _source.frameCount();
-}
-
-
-bool
-AnalysisDataProxy::getDataWErr(int index, real *x, real *dx,
-                               const real **y, const real **dy,
-                               const bool **missing) const
+AnalysisDataFrameRef
+AnalysisDataProxy::tryGetDataFrameInternal(int index) const
 {
-    bool bExists = _source.getDataWErr(index, x, dx, y, dy, missing);
-    if (bExists)
+    AnalysisDataFrameRef frame = _source.tryGetDataFrame(index);
+    if (!frame.isValid())
     {
-        if (y && *y)
-        {
-            *y += _col;
-        }
-        if (dy && *dy)
-        {
-            *dy += _col;
-        }
-        if (missing && *missing)
-        {
-            *missing += _col;
-        }
+        return AnalysisDataFrameRef();
     }
-    return bExists;
+    return AnalysisDataFrameRef(frame, _col, _span);
 }
 
 
 bool
-AnalysisDataProxy::requestStorage(int nframes)
+AnalysisDataProxy::requestStorageInternal(int nframes)
 {
     return _source.requestStorage(nframes);
 }
index 2877c6f87bb11dbdbc9d8bdffddf8d737b016d48..6b36888270b964e78534e64f8655e7896c25d062 100644 (file)
@@ -65,12 +65,6 @@ class AnalysisDataProxy : public AbstractAnalysisData,
          */
         AnalysisDataProxy(int col, int span, AbstractAnalysisData *data);
 
-        virtual int frameCount() const;
-        virtual bool getDataWErr(int index, real *x, real *dx,
-                                 const real **y, const real **dy,
-                                 const bool **missing = 0) const;
-        virtual bool requestStorage(int nframes = -1);
-
         virtual int flags() const;
 
         virtual void dataStarted(AbstractAnalysisData *data);
@@ -80,6 +74,9 @@ class AnalysisDataProxy : public AbstractAnalysisData,
         virtual void dataFinished();
 
     private:
+        virtual AnalysisDataFrameRef tryGetDataFrameInternal(int index) const;
+        virtual bool requestStorageInternal(int nframes);
+
         AbstractAnalysisData   &_source;
         int                     _col;
         int                     _span;
index 6c7c221520987c320297d908ec9831603081fc49..45b8a78e3e0f0ef965dc9628421e05173d2dcb3a 100644 (file)
@@ -106,17 +106,15 @@ AnalysisDataDisplacementModule::setMSDHistogram(AnalysisDataBinAverageModule *hi
 }
 
 
-bool
-AnalysisDataDisplacementModule::getDataWErr(int index, real *x, real *dx,
-                                            const real **y, const real **dy,
-                                            const bool **present) const
+AnalysisDataFrameRef
+AnalysisDataDisplacementModule::tryGetDataFrameInternal(int /*index*/) const
 {
-    return false;
+    return AnalysisDataFrameRef();
 }
 
 
 bool
-AnalysisDataDisplacementModule::requestStorage(int nframes)
+AnalysisDataDisplacementModule::requestStorageInternal(int /*nframes*/)
 {
     return false;
 }
index b4e715fef5c786524afacf8d4fcbb4fa3b221e72..9ad65eeb5ba4cd833cfb584547e8d8c01797f7ad 100644 (file)
@@ -79,11 +79,6 @@ class AnalysisDataDisplacementModule : public AbstractAnalysisData,
          */
         void setMSDHistogram(AnalysisDataBinAverageModule *histm);
 
-        virtual bool getDataWErr(int index, real *x, real *dx,
-                                 const real **y, const real **dy,
-                                 const bool **present = 0) const;
-        virtual bool requestStorage(int nframes = -1);
-
         virtual int flags() const;
 
         virtual void dataStarted(AbstractAnalysisData *data);
@@ -93,6 +88,9 @@ class AnalysisDataDisplacementModule : public AbstractAnalysisData,
         virtual void dataFinished();
 
     private:
+        virtual AnalysisDataFrameRef tryGetDataFrameInternal(int index) const;
+        virtual bool requestStorageInternal(int nframes);
+
         class Impl;
 
         Impl                   *_impl;
index 42c958b97279d7e1fe613fdf5f1f99e1d41f9588..2c1bd82bbb0dfdd0b57140c3d2ced7dc071cf164 100644 (file)
@@ -235,7 +235,7 @@ TEST_F(AnalysisDataTest, CanAddModuleAfterStoredData)
     gmx::test::AnalysisDataTestInput input(inputdata);
     gmx::AnalysisData data;
     data.setColumns(input.columnCount());
-    data.requestStorage(-1);
+    ASSERT_TRUE(data.requestStorage(-1));
 
     ASSERT_NO_THROW(presentAllData(input, &data));
     ASSERT_NO_THROW(addStaticCheckerModule(input, &data));
index bb67be237bfa5dbaf9a323ca0cb3ff6c6f0a014b..e44ddac3511dd50e5e1cef5b5a469b6db9775f8a 100644 (file)
@@ -293,7 +293,7 @@ class AnalysisDataTestFixture : public ::testing::Test
          *
          * Works like addStaticCheckerModule(), except that in addition, for
          * each frame, the mock module also checks that previous frames can be
-         * accessed using AbstractAnalysisData::getDataWErr().  In the
+         * accessed using AbstractAnalysisData::getDataFrame().  In the
          * AnalysisDataModuleInterface::dataStarted() callback, the mock module
          * calls AbstractAnalysisData::requestStorage() with \p storageCount as
          * the parameter.
index 5ceb0ab3942c25cf8b47506d37535493e59c6d61..63b6b2d037fa8aecc4d9fa802d133713f9937495 100644 (file)
@@ -133,20 +133,11 @@ void checkPoints(const AnalysisDataPointSetRef &points,
     }
 }
 
-void checkPoints(int firstcol, int n, const real *y,
-                 const AnalysisDataTestInputPointSet &points)
+void checkFrame(const AnalysisDataFrameRef &frame,
+                const AnalysisDataTestInputFrame &refFrame)
 {
-    for (int i = 0; i < n; ++i)
-    {
-        EXPECT_FLOAT_EQ(points.y(firstcol + i), y[i]);
-    }
-}
-
-void checkFrame(int index, real x, real dx, int firstcol, int n, const real *y,
-                const AnalysisDataTestInputFrame &frame)
-{
-    checkHeader(AnalysisDataFrameHeader(index, x, dx), frame);
-    checkPoints(firstcol, n, y, frame.points());
+    checkHeader(frame.header(), refFrame);
+    checkPoints(frame.points(), refFrame.points(), 0);
 }
 
 /*! \internal \brief
@@ -248,7 +239,7 @@ class DataStorageRequester
         //! Function call operator for the functor.
         void operator()(AbstractAnalysisData *data) const
         {
-            data->requestStorage(count_);
+            EXPECT_TRUE(data->requestStorage(count_));
         }
 
     private:
@@ -297,25 +288,17 @@ class StaticDataPointsStorageChecker
             EXPECT_EQ(data_->columnCount(), points.columnCount());
             checkHeader(points.header(), data_->frame(frameIndex_));
             checkPoints(points, data_->frame(frameIndex_).points(), 0);
-            for (int past = 0;
+            for (int past = 1;
                  (storageCount_ < 0 || past <= storageCount_) && past <= frameIndex_;
                  ++past)
             {
                 int   index = frameIndex_ - past;
-                real  pastx, pastdx;
-                const real *pasty;
                 SCOPED_TRACE(formatString("Checking storage of frame %d", index));
-                ASSERT_TRUE(source_->getDataWErr(index,
-                                                 &pastx, &pastdx, &pasty, NULL));
-                checkFrame(index, pastx, pastdx, 0, data_->columnCount(), pasty,
-                           data_->frame(index));
-                if (past > 0)
-                {
-                    ASSERT_TRUE(source_->getDataWErr(-past,
-                                                     &pastx, &pastdx, &pasty, NULL));
-                    checkFrame(index, pastx, pastdx, 0, data_->columnCount(), pasty,
-                               data_->frame(index));
-                }
+                ASSERT_NO_THROW({
+                    AnalysisDataFrameRef frame = source_->getDataFrame(index);
+                    ASSERT_TRUE(frame.isValid());
+                    checkFrame(frame, data_->frame(index));
+                });
             }
         }
 
index b0884e66a1354c90e59fdaf820718a9b77356c20..a12532cead38729d056d039ec95bf6d0b0d47159 100644 (file)
@@ -154,11 +154,8 @@ Distance::finishAnalysis(int /*nframes*/)
 void
 Distance::writeOutput()
 {
-    const real *ave;
-
-    _avem->getData(0, NULL, &ave, NULL);
-    fprintf(stderr, "Average distance: %f\n", ave[0]);
-    fprintf(stderr, "Std. deviation:   %f\n", ave[1]);
+    fprintf(stderr, "Average distance: %f\n", _avem->average(0));
+    fprintf(stderr, "Std. deviation:   %f\n", _avem->stddev(0));
 }