Merge "Merge branch release-2018"
authorBerk Hess <hess@kth.se>
Fri, 24 Aug 2018 15:44:02 +0000 (17:44 +0200)
committerBerk Hess <hess@kth.se>
Fri, 24 Aug 2018 15:44:02 +0000 (17:44 +0200)
17 files changed:
src/gromacs/domdec/ga2la.cpp
src/gromacs/domdec/ga2la.h
src/gromacs/domdec/hashedmap.h
src/gromacs/domdec/tests/CMakeLists.txt
src/gromacs/domdec/tests/hashedmap.cpp [new file with mode: 0644]
src/gromacs/gmxpreprocess/fflibutil.cpp
src/gromacs/gmxpreprocess/fflibutil.h
src/gromacs/gmxpreprocess/gen_vsite.cpp
src/gromacs/gmxpreprocess/h_db.cpp
src/gromacs/gmxpreprocess/nm2type.cpp
src/gromacs/gmxpreprocess/pdb2gmx.cpp
src/gromacs/gmxpreprocess/pdb2top.cpp
src/gromacs/gmxpreprocess/resall.cpp
src/gromacs/gmxpreprocess/resall.h
src/gromacs/gmxpreprocess/ter_db.cpp
src/gromacs/gmxpreprocess/xlate.cpp
src/gromacs/gmxpreprocess/xlate.h

index 82e8f3f95c4b976b217d8b5087ad38330b0a33b8..071e9697e48fb76cbec532e4fcec176b1ffddf10 100644 (file)
@@ -69,20 +69,6 @@ static bool directListIsFaster(int numAtomsTotal,
             numAtomsTotal <= numAtomsLocal*c_memoryRatioHashedVersusDirect);
 }
 
-/*! \brief Returns the base size of the hash table
- *
- * Make the direct list twice as long as the number of local atoms.
- * The fraction of entries in the list with:
- * 0   size lists: e^-1/f
- * >=1 size lists: 1 - e^-1/f
- * where f is: the direct list length / nr. of local atoms
- * The fraction of atoms not in the direct list is: 1-f(1-e^-1/f).
- */
-static int baseTableSizeForHashTable(int numAtomsLocal)
-{
-    return 2*numAtomsLocal;
-}
-
 gmx_ga2la_t::gmx_ga2la_t(int numAtomsTotal,
                          int numAtomsLocal) :
     usingDirect_(directListIsFaster(numAtomsTotal, numAtomsLocal))
@@ -93,6 +79,6 @@ gmx_ga2la_t::gmx_ga2la_t(int numAtomsTotal,
     }
     else
     {
-        new(&(data_.hashed))HashedMap<Entry>(baseTableSizeForHashTable(numAtomsLocal));
+        new(&(data_.hashed)) gmx::HashedMap<Entry>(numAtomsLocal);
     }
 }
index 76dfd7675e1c24e7603efe47f4d1d1bc0d3c6130..ff1e514683a40717a319218634437a955dfebff4 100644 (file)
@@ -168,8 +168,8 @@ class gmx_ga2la_t
     private:
         union Data
         {
-            std::vector<Entry> direct;
-            HashedMap<Entry>   hashed;
+            std::vector<Entry>    direct;
+            gmx::HashedMap<Entry> hashed;
             // constructor and destructor function in parent class
             Data()  {}
             ~Data() {}
index f74ce4e96e0e24fbd5bed02015838a03ba99d221..8370361a0e49668111430a24f6cf20d5cf4778c1 100644 (file)
 #ifndef GMX_DOMDEC_HASHEDMAP_H
 #define GMX_DOMDEC_HASHEDMAP_H
 
+#include <climits>
+
 #include <algorithm>
 #include <vector>
 
 #include "gromacs/compat/utility.h"
 #include "gromacs/utility/basedefinitions.h"
-#include "gromacs/utility/gmxassert.h"
+#include "gromacs/utility/exceptions.h"
+
+namespace gmx
+{
 
 /*! \libinternal \brief Unordered key to value mapping
  *
- * Efficiently manages mapping from non-negative integer keys to values.
+ * Efficiently manages mapping from integer keys to values.
  * Note that this basically implements a subset of the functionality of
  * std::unordered_map, but is an order of magnitude faster.
  */
@@ -72,29 +77,77 @@ class HashedMap
             int  next = -1;  /**< Index in the list of the next element with the same hash, -1 if none */
         };
 
+        /*! \brief The table size is set to at least this factor time the nr of keys */
+        static constexpr float c_relTableSizeSetMin       = 1.5;
+        /*! \brief Threshold for increasing the table size */
+        static constexpr float c_relTableSizeThresholdMin = 1.3;
+        /*! \brief Threshold for decreasing the table size */
+        static constexpr float c_relTableSizeThresholdMax = 3.5;
+
+        /*! \brief Resizes the table
+         *
+         * \param[in] numElementsEstimate  An estimate of the number of elements that will be stored
+         */
+        void resize(int numElementsEstimate)
+        {
+            GMX_RELEASE_ASSERT(numElements_ == 0, "Table needs to be empty for resize");
+
+            /* The fraction of table entries with 0   size lists is e^-f.
+             * The fraction of table entries with >=1 size lists is 1 - e^-f
+             * where f is: the #elements / tableSize
+             * The fraction of elements not in the direct list is: 1 - (1 - e^-f)/f.
+             * Thus the optimal table size is roughly double #elements.
+             */
+            /* Make the hash table a power of 2 and at least 1.5 * #elements */
+            int tableSize = 64;
+            while (tableSize <= INT_MAX/2 &&
+                   numElementsEstimate*c_relTableSizeSetMin > tableSize)
+            {
+                tableSize *= 2;
+            }
+            table_.resize(tableSize);
+
+            /* Table size is a power of 2, so a binary mask gives the hash */
+            bitMask_                        = tableSize - 1;
+            startIndexForSpaceForListEntry_ = tableSize;
+        }
+
     public:
         /*! \brief Constructor
          *
-         * \param[in] baseTableSize  The size of the base table, optimal is around twice the number of expected entries
+         * \param[in] numElementsEstimate  An estimate of the number of elements that will be stored, used for optimizing initial performance
+         *
+         * Note that the estimate of the number of elements is only relevant
+         * for the performance up until the first call to clear(), after which
+         * table size is optimized based on the actual number of elements.
          */
-        HashedMap(int baseTableSize) :
-            table_(baseTableSize),
-            mod_(baseTableSize),
-            startSpaceSearch_(baseTableSize)
+        HashedMap(int numElementsEstimate)
+        {
+            resize(numElementsEstimate);
+        }
+
+        /*! \brief Returns the number of elements */
+        int size() const
+        {
+            return numElements_;
+        }
+
+        /*! \brief Returns the number of buckets, i.e. the number of possible hashes */
+        int bucket_count() const
         {
+            return bitMask_ + 1;
         }
 
-        /*! \brief Inserts entry, key should not already be present (checked by assertion)
+        /*! \brief Inserts entry, key should not already be present
          *
          * \param[in] key    The key for the entry
          * \param[in] value  The value for the entry
+         * \throws InvalidInputError from a debug build when attempting to insert a duplicate key
          */
         void insert(int      key,
                     const T &value)
         {
-            GMX_ASSERT(key >= 0, "Only keys >= 0 are supported");
-
-            size_t ind = key % mod_;
+            size_t ind = (key & bitMask_);
 
             if (table_[ind].key >= 0)
             {
@@ -102,14 +155,25 @@ class HashedMap
                  * If we find the matching key, return the value.
                  */
                 int ind_prev = ind;
-                GMX_ASSERT(table_[ind_prev].key != key, "The key to be inserted should not be present");
+// Note: This is performance critical, so we only throw in debug mode
+#ifndef NDEBUG
+                if (table_[ind_prev].key == key)
+                {
+                    GMX_THROW(InvalidInputError("Attempt to insert duplicate key"));
+                }
+#endif
                 while (table_[ind_prev].next >= 0)
                 {
                     ind_prev = table_[ind_prev].next;
-                    GMX_ASSERT(table_[ind_prev].key != key, "The key to be inserted should not be present");
+#ifndef NDEBUG
+                    if (table_[ind_prev].key == key)
+                    {
+                        GMX_THROW(InvalidInputError("Attempt to insert duplicate key"));
+                    }
+#endif
                 }
-                /* Search for space in the array */
-                ind = startSpaceSearch_;
+                /* Search for space in table_ */
+                ind = startIndexForSpaceForListEntry_;
                 while (ind < table_.size() && table_[ind].key >= 0)
                 {
                     ind++;
@@ -119,23 +183,25 @@ class HashedMap
                 {
                     table_.resize(table_.size() + 1);
                 }
-                table_[ind_prev].next = ind;
+                table_[ind_prev].next           = ind;
 
-                startSpaceSearch_ = ind + 1;
+                startIndexForSpaceForListEntry_ = ind + 1;
             }
 
-            table_[ind].key   = key;
-            table_[ind].value = value;
+            table_[ind].key    = key;
+            table_[ind].value  = value;
+
+            numElements_      += 1;
         }
 
-        /*! \brief Delete the entry for key \p key
+        /*! \brief Delete the entry for key \p key, when present
          *
          * \param[in] key  The key
          */
         void erase(int key)
         {
             int ind_prev = -1;
-            int ind      = key % mod_;
+            int ind      = (key & bitMask_);
             do
             {
                 if (table_[ind].key == key)
@@ -147,13 +213,15 @@ class HashedMap
                         /* This index is a linked entry, so we free an entry.
                          * Check if we are creating the first empty space.
                          */
-                        if (ind < startSpaceSearch_)
+                        if (ind < startIndexForSpaceForListEntry_)
                         {
-                            startSpaceSearch_ = ind;
+                            startIndexForSpaceForListEntry_ = ind;
                         }
                     }
-                    table_[ind].key  = -1;
-                    table_[ind].next = -1;
+                    table_[ind].key   = -1;
+                    table_[ind].next  = -1;
+
+                    numElements_     -= 1;
 
                     return;
                 }
@@ -177,7 +245,7 @@ class HashedMap
          */
         const T *find(int key) const
         {
-            int ind = key % mod_;
+            int ind = (key & bitMask_);
             do
             {
                 if (table_[ind].key == key)
@@ -191,21 +259,45 @@ class HashedMap
             return nullptr;
         }
 
-        /*! \brief Clear all the entries in the list */
+        /*! \brief Clear all the entries in the list
+         *
+         * Also optimizes the size of the table based on the current
+         * number of elements stored.
+         */
         void clear()
         {
+            const int oldNumElements = numElements_;
+
             for (hashEntry &entry : table_)
             {
                 entry.key  = -1;
                 entry.next = -1;
             }
-            startSpaceSearch_ = mod_;
+            startIndexForSpaceForListEntry_ = bucket_count();
+            numElements_                    = 0;
+
+            /* Resize the hash table when the occupation is far from optimal.
+             * Do not resize with 0 elements to avoid minimal size when clear()
+             * is called twice in a row.
+             */
+            if (oldNumElements > 0 && (oldNumElements*c_relTableSizeThresholdMax < bucket_count() ||
+                                       oldNumElements*c_relTableSizeThresholdMin > bucket_count()))
+            {
+                resize(oldNumElements);
+            }
         }
 
     private:
-        std::vector<hashEntry> table_;            /**< The hash table list */
-        int                    mod_;              /**< The hash size */
-        int                    startSpaceSearch_; /**< Index in lal at which to start looking for empty space */
+        /*! \brief The hash table list */
+        std::vector<hashEntry> table_;
+        /*! \brief The bit mask for computing the hash of a key */
+        int                    bitMask_                        = 0;
+        /*! \brief Index in table_ at which to start looking for empty space for a new linked list entry */
+        int                    startIndexForSpaceForListEntry_ = 0;
+        /*! \brief The number of elements currently stored in the table */
+        int                    numElements_                    = 0;
 };
 
+} // namespace gmx
+
 #endif
index c4ee524f403f145c8febb4cb4e010aeabad5d023..478f578e355e54fff9ff0439f4ba8d5e48aac5bd 100644 (file)
@@ -33,4 +33,5 @@
 # the research papers on the package. Check out http://www.gromacs.org.
 
 gmx_add_unit_test(DomDecTests domdec-test
+            hashedmap.cpp
             localatomsetmanager.cpp)
diff --git a/src/gromacs/domdec/tests/hashedmap.cpp b/src/gromacs/domdec/tests/hashedmap.cpp
new file mode 100644 (file)
index 0000000..d84725b
--- /dev/null
@@ -0,0 +1,198 @@
+/*
+ * This file is part of the GROMACS molecular simulation package.
+ *
+ * Copyright (c) 2018, 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 for the HashedMap class.
+ *
+ * \author berk Hess <hess@kth.se>
+ * \ingroup module_domdec
+ */
+#include "gmxpre.h"
+
+#include "gromacs/domdec/hashedmap.h"
+
+#include <gtest/gtest.h>
+
+#include "testutils/testasserts.h"
+
+namespace
+{
+
+/*! \brief Checks that the key is found and if so also checks the value */
+void checkFinds(const gmx::HashedMap<char> &map,
+                int                         key,
+                char                        value)
+{
+    const char *pointer = map.find(key);
+    EXPECT_FALSE(pointer == nullptr);
+    if (pointer)
+    {
+        EXPECT_EQ(*pointer, value);
+    }
+}
+
+/*! \brief Checks that the key is not found */
+void checkDoesNotFind(const gmx::HashedMap<char> &map,
+                      int                         key)
+{
+    const char *pointer = map.find(key);
+    EXPECT_TRUE(pointer == nullptr);
+}
+
+TEST(HashedMap, InsertsFinds)
+{
+    gmx::HashedMap<char> map(2);
+
+    map.insert(10, 'a');
+    map.insert(5,  'b');
+    map.insert(7,  'c');
+
+    checkFinds(map, 10, 'a');
+    checkFinds(map, 5,  'b');
+    checkFinds(map, 7,  'c');
+    checkDoesNotFind(map, 4);
+}
+
+TEST(HashedMap, NegativeKeysWork)
+{
+    gmx::HashedMap<char> map(5);
+
+    map.insert(-1, 'a');
+    map.insert(1,  'b');
+    map.insert(-3, 'c');
+
+    checkFinds(map, -1, 'a');
+    checkFinds(map, 1,  'b');
+    checkFinds(map, -3, 'c');
+}
+
+TEST(HashedMap, InsertsErases)
+{
+    gmx::HashedMap<char> map(3);
+
+    map.insert(10, 'a');
+    map.insert(5,  'b');
+    map.insert(7,  'c');
+
+    checkFinds(map, 10, 'a');
+    map.erase(10);
+    checkDoesNotFind(map, 10);
+}
+
+TEST(HashedMap, Clears)
+{
+    gmx::HashedMap<char> map(3);
+
+    map.insert(10, 'a');
+    map.insert(5,  'b');
+    map.insert(7,  'c');
+
+    map.clear();
+    checkDoesNotFind(map, 10);
+    checkDoesNotFind(map, 5);
+    checkDoesNotFind(map, 7);
+}
+
+// Check that entries with the same hash are handled correctly
+TEST(HashedMap, LinkedEntries)
+{
+    // HashedMap uses bit masking, so keys that differ by exactly
+    // a power of 2 larger than the table size will have the same hash
+
+    gmx::HashedMap<char> map(20);
+
+    const int            largePowerOf2 = 2048;
+
+    map.insert(3 + 0*largePowerOf2, 'a');
+    map.insert(3 + 1*largePowerOf2, 'b');
+    map.insert(3 + 2*largePowerOf2, 'c');
+
+    checkFinds(map, 3 + 0*largePowerOf2, 'a');
+    checkFinds(map, 3 + 1*largePowerOf2, 'b');
+    checkFinds(map, 3 + 2*largePowerOf2, 'c');
+
+    // Erase the middle entry in the linked list
+    map.erase(3 + 1*largePowerOf2);
+
+    checkFinds(map, 3 + 0*largePowerOf2, 'a');
+    checkDoesNotFind(map, 3 + 1*largePowerOf2);
+    checkFinds(map, 3 + 2*largePowerOf2, 'c');
+}
+
+// HashedMap only throws in debug mode, so only test in debug mode
+#ifndef NDEBUG
+
+TEST(HashedMap, CatchesDuplicateKey)
+{
+    gmx::HashedMap<char> map(15);
+
+    map.insert(10, 'a');
+    map.insert(5,  'b');
+    EXPECT_THROW_GMX(map.insert(10, 'c'), gmx::InvalidInputError);
+}
+
+#endif // NDEBUG
+
+// Check the table is resized after clear()
+TEST(HashedMap, ResizesTable)
+{
+    gmx::HashedMap<char> map(1);
+
+    // This test assumes the minimum bucket count is 64 or less
+    EXPECT_LT(map.bucket_count(), 128);
+
+    for (int i = 0; i < 60; i++)
+    {
+        map.insert(2*i + 3, 'a');
+    }
+    EXPECT_LT(map.bucket_count(), 128);
+
+    // Check that the table size is double #elements after clear()
+    map.clear();
+    EXPECT_EQ(map.bucket_count(), 128);
+
+    // Check that calling clear() a second time does not resize
+    map.clear();
+    EXPECT_EQ(map.bucket_count(), 128);
+
+    map.insert(2, 'b');
+    EXPECT_EQ(map.bucket_count(), 128);
+
+    // Check that calling clear with 1 elements sizes down
+    map.clear();
+    EXPECT_LT(map.bucket_count(), 128);
+}
+
+}      // namespace
index 3b55c4e836406c556b8e2b9f033baf74d684a8c2..614608e326780860cc1a1cb1490e9bf25372f436 100644 (file)
@@ -96,10 +96,9 @@ void fflib_filename_base(const char *filename, char *filebase, int maxlen)
     }
 }
 
-int fflib_search_file_end(const char *ffdir,
-                          const char *file_end,
-                          bool        bFatalError,
-                          char     ***filenames)
+std::vector<std::string> fflib_search_file_end(const char *ffdir,
+                                               const char *file_end,
+                                               bool        bFatalError)
 {
     try
     {
@@ -115,19 +114,11 @@ int fflib_search_file_end(const char *ffdir,
                                     file_end, ffdir);
             GMX_THROW(gmx::InvalidInputError(message));
         }
-        const int count = static_cast<int>(result.size());
-        for (int i = 0; i < count; ++i)
+        for (std::string &filename : result)
         {
-            result[i] = gmx::Path::join(ffdir, result[i]);
+            filename = gmx::Path::join(ffdir, filename);
         }
-        char    **fns;
-        snew(fns, count);
-        for (int i = 0; i < count; ++i)
-        {
-            fns[i] = gmx_strdup(result[i].c_str());
-        }
-        *filenames = fns;
-        return count;
+        return result;
     }
     GMX_CATCH_ALL_AND_EXIT_WITH_FATAL_ERROR;
 }
@@ -167,11 +158,11 @@ std::vector<gmx::DataFileInfo> fflib_enumerate_forcefields()
     return result;
 }
 
-bool fflib_fexist(const char *file)
+bool fflib_fexist(const std::string &file)
 {
     char *file_fullpath;
 
-    file_fullpath = low_gmxlibfn(file, TRUE, FALSE);
+    file_fullpath = low_gmxlibfn(file.c_str(), TRUE, FALSE);
 
     if (file_fullpath == nullptr)
     {
@@ -186,12 +177,12 @@ bool fflib_fexist(const char *file)
 }
 
 
-FILE *fflib_open(const char *file)
+FILE *fflib_open(const std::string &file)
 {
     char *file_fullpath;
     FILE *fp;
 
-    file_fullpath = gmxlibfn(file);
+    file_fullpath = gmxlibfn(file.c_str());
     fprintf(stderr, "Opening force field file %s\n", file_fullpath);
     fp = gmx_ffopen(file_fullpath, "r");
     sfree(file_fullpath);
index 413a898b6dded9024326fa5573946c27da49691c..6aeae89ce00fc9803fe5995d5f61a00e2b99960d 100644 (file)
@@ -38,6 +38,7 @@
 
 #include <stdio.h>
 
+#include <string>
 #include <vector>
 
 #include "gromacs/utility/basedefinitions.h"
@@ -63,19 +64,18 @@ void fflib_filename_base(const char *filename, char *filebase, int maxlen);
  * base should be at least of size maxlen.
  */
 
-int fflib_search_file_end(const char *ffdir,
-                          const char *file_end,
-                          bool        bFatalError,
-                          char     ***filenames);
+std::vector<std::string> fflib_search_file_end(const char *ffdir,
+                                               const char *file_end,
+                                               bool        bFatalError);
 /* Search for files ending on file_end in the force field directory fflib.
  * fflib should be in the GROMACS lib.path.
  * Return the number of files and the file names in filenames.
  */
 
-bool fflib_fexist(const char *file);
+bool fflib_fexist(const std::string &file);
 /* Check if a file exists in the force field library */
 
-FILE *fflib_open(const char *file);
+FILE *fflib_open(const std::string &file);
 /* Open force field library file "file" for reading.
  * "file" should contain the whole path to the force field library,
  * either absolute or relative to the current dir.
index 2052274fe56089575301dd2d77f0f19b889b6874..8f6f99eed5b2c91c0e07989e3bce0a5569866c38 100644 (file)
@@ -43,6 +43,9 @@
 #include <cstdlib>
 #include <cstring>
 
+#include <string>
+#include <vector>
+
 #include "gromacs/fileio/pdbio.h"
 #include "gromacs/gmxpreprocess/add_par.h"
 #include "gromacs/gmxpreprocess/fflibutil.h"
@@ -1535,8 +1538,6 @@ void do_vsites(int nrtp, t_restp rtp[], gpp_atomtype_t atype,
     t_params         *params;
     char           ***newatomname;
     char             *resnm = nullptr;
-    int               ndb, f;
-    char            **db;
     int               nvsiteconf, nvsitetop, cmplength;
     bool              isN, planarN, bFound;
     gmx_residuetype_t*rt;
@@ -1596,17 +1597,15 @@ void do_vsites(int nrtp, t_restp rtp[], gpp_atomtype_t atype,
         fprintf(debug, "# # # VSITES # # #\n");
     }
 
-    ndb           = fflib_search_file_end(ffdir, ".vsd", FALSE, &db);
+    std::vector<std::string> db = fflib_search_file_end(ffdir, ".vsd", FALSE);
     nvsiteconf    = 0;
     vsiteconflist = nullptr;
     nvsitetop     = 0;
     vsitetop      = nullptr;
-    for (f = 0; f < ndb; f++)
+    for (const auto &filename : db)
     {
-        read_vsite_database(db[f], &vsiteconflist, &nvsiteconf, &vsitetop, &nvsitetop);
-        sfree(db[f]);
+        read_vsite_database(filename.c_str(), &vsiteconflist, &nvsiteconf, &vsitetop, &nvsitetop);
     }
-    sfree(db);
 
     bFirstWater = TRUE;
     nvsite      = 0;
@@ -1872,7 +1871,7 @@ void do_vsites(int nrtp, t_restp rtp[], gpp_atomtype_t atype,
 
                     if (ch == nullptr)
                     {
-                        if (ndb > 0)
+                        if (!db.empty())
                         {
                             gmx_fatal(FARGS, "Can't find dummy mass for type %s bonded to type %s in the virtual site database (.vsd files). Add it to the database!\n", tpname, nexttpname);
                         }
index c8d2bc623f099997bb09f04d7b99d1d2aa60a13a..71da9aece88da71844c2c2e3266763ae4fde770c 100644 (file)
@@ -42,6 +42,9 @@
 #include <cstdlib>
 #include <cstring>
 
+#include <string>
+#include <vector>
+
 #include "gromacs/gmxpreprocess/fflibutil.h"
 #include "gromacs/gmxpreprocess/notset.h"
 #include "gromacs/utility/arraysize.h"
@@ -197,23 +200,17 @@ static void read_h_db_file(const char *hfn, int *nahptr, t_hackblock **ah)
 
 int read_h_db(const char *ffdir, t_hackblock **ah)
 {
-    int    nhdbf, f;
-    char **hdbf;
-    int    nah;
-
     /* Read the hydrogen database file(s).
      * Do not generate an error when no files are found.
      */
-    nhdbf = fflib_search_file_end(ffdir, ".hdb", FALSE, &hdbf);
-    nah   = 0;
+
+    std::vector<std::string> hdbf = fflib_search_file_end(ffdir, ".hdb", FALSE);
+    int nah                       = 0;
     *ah   = nullptr;
-    for (f = 0; f < nhdbf; f++)
+    for (const auto &filename : hdbf)
     {
-        read_h_db_file(hdbf[f], &nah, ah);
-        sfree(hdbf[f]);
+        read_h_db_file(filename.c_str(), &nah, ah);
     }
-    sfree(hdbf);
-
     return nah;
 }
 
index 0bb7da27ffa5c1a8bc4e20c0f96b974f70880d0e..a952ee5d1c85480fb647939768448bce0c49160a 100644 (file)
@@ -42,6 +42,8 @@
 #include <cstring>
 
 #include <algorithm>
+#include <string>
+#include <vector>
 
 #include "gromacs/fileio/confio.h"
 #include "gromacs/gmxpreprocess/fflibutil.h"
@@ -58,7 +60,7 @@
 #include "gromacs/utility/futil.h"
 #include "gromacs/utility/smalloc.h"
 
-static void rd_nm2type_file(const char *fn, int *nnm, t_nm2type **nmp)
+static void rd_nm2type_file(const std::string &filename, int *nnm, t_nm2type **nmp)
 {
     FILE         *fp;
     bool          bCont;
@@ -69,10 +71,10 @@ static void rd_nm2type_file(const char *fn, int *nnm, t_nm2type **nmp)
     double        qq, mm;
     t_nm2type    *nm2t = nullptr;
 
-    fp = fflib_open(fn);
+    fp = fflib_open(filename);
     if (nullptr == fp)
     {
-        gmx_fatal(FARGS, "Can not find %s in library directory", fn);
+        gmx_fatal(FARGS, "Can not find %s in library directory", filename.c_str());
     }
 
     nnnm = *nnm;
@@ -132,20 +134,13 @@ static void rd_nm2type_file(const char *fn, int *nnm, t_nm2type **nmp)
 
 t_nm2type *rd_nm2type(const char *ffdir, int *nnm)
 {
-    int        nff, f;
-    char     **ff;
-    t_nm2type *nm;
-
-    nff  = fflib_search_file_end(ffdir, ".n2t", FALSE, &ff);
+    std::vector<std::string> ff  = fflib_search_file_end(ffdir, ".n2t", FALSE);
     *nnm = 0;
-    nm   = nullptr;
-    for (f = 0; f < nff; f++)
+    t_nm2type               *nm = nullptr;
+    for (const auto &filename : ff)
     {
-        rd_nm2type_file(ff[f], nnm, &nm);
-        sfree(ff[f]);
+        rd_nm2type_file(filename, nnm, &nm);
     }
-    sfree(ff);
-
     return nm;
 }
 
index 2a1a219cf19a6a45b39099602be0b9586aa8de83..d3310366284802004a65cd2dd40a7e9891f63d51 100644 (file)
@@ -44,6 +44,8 @@
 #include <ctime>
 
 #include <algorithm>
+#include <string>
+#include <vector>
 
 #include "gromacs/commandline/pargs.h"
 #include "gromacs/fileio/confio.h"
@@ -1387,10 +1389,6 @@ int gmx_pdb2gmx(int argc, char *argv[])
     char              ffname[STRLEN], suffix[STRLEN], buf[STRLEN];
     char             *watermodel;
     const char       *watres;
-    int               nrtpf;
-    char            **rtpf;
-    int               nrrn;
-    char            **rrn;
     int               nrtprename;
     rtprename_t      *rtprename = nullptr;
     int               nah, nNtdb, nCtdb, ntdblist;
@@ -1599,18 +1597,16 @@ int gmx_pdb2gmx(int argc, char *argv[])
     gmx_residuetype_init(&rt);
 
     /* Read residue renaming database(s), if present */
-    nrrn = fflib_search_file_end(ffdir, ".r2b", FALSE, &rrn);
+    std::vector<std::string> rrn = fflib_search_file_end(ffdir, ".r2b", FALSE);
 
     nrtprename = 0;
     rtprename  = nullptr;
-    for (i = 0; i < nrrn; i++)
+    for (const auto &filename : rrn)
     {
-        fp = fflib_open(rrn[i]);
-        read_rtprename(rrn[i], fp, &nrtprename, &rtprename);
+        fp = fflib_open(filename);
+        read_rtprename(filename.c_str(), fp, &nrtprename, &rtprename);
         gmx_ffclose(fp);
-        sfree(rrn[i]);
     }
-    sfree(rrn);
 
     /* Add all alternative names from the residue renaming database to the list of recognized amino/nucleic acids. */
     for (i = 0; i < nrtprename; i++)
@@ -1883,15 +1879,13 @@ int gmx_pdb2gmx(int argc, char *argv[])
 
     /* read residue database */
     printf("Reading residue database... (%s)\n", forcefield);
-    nrtpf = fflib_search_file_end(ffdir, ".rtp", TRUE, &rtpf);
+    std::vector<std::string> rtpf = fflib_search_file_end(ffdir, ".rtp", TRUE);
     nrtp  = 0;
     restp = nullptr;
-    for (i = 0; i < nrtpf; i++)
+    for (const auto &filename : rtpf)
     {
-        read_resall(rtpf[i], &nrtp, &restp, atype, &symtab, FALSE);
-        sfree(rtpf[i]);
+        read_resall(filename.c_str(), &nrtp, &restp, atype, &symtab, FALSE);
     }
-    sfree(rtpf);
     if (bNewRTP)
     {
         /* Not correct with multiple rtp input files with different bonded types */
index d9175be77478dec70adff6130de442551df7310a..fa904f8ae38a1d3b7f817c76bbc1fb9404ec71fd 100644 (file)
@@ -357,7 +357,6 @@ void choose_watermodel(const char *wmsel, const char *ffdir,
                        char **watermodel)
 {
     const char *fn_watermodels = "watermodels.dat";
-    char        fn_list[STRLEN];
     FILE       *fp;
     char        buf[STRLEN];
     int         nwm, sel, i;
@@ -377,9 +376,8 @@ void choose_watermodel(const char *wmsel, const char *ffdir,
         return;
     }
 
-    sprintf(fn_list, "%s%c%s", ffdir, DIR_SEPARATOR, fn_watermodels);
-
-    if (!fflib_fexist(fn_list))
+    std::string filename = gmx::Path::join(ffdir, fn_watermodels);
+    if (!fflib_fexist(filename))
     {
         fprintf(stderr, "No file '%s' found, will not include a water model\n",
                 fn_watermodels);
@@ -388,7 +386,7 @@ void choose_watermodel(const char *wmsel, const char *ffdir,
         return;
     }
 
-    fp = fflib_open(fn_list);
+    fp = fflib_open(filename);
     printf("\nSelect the Water Model:\n");
     nwm   = 0;
     model = nullptr;
index 00af8b42774724d1ce01b3435a8733d82cd07a0e..a2294135cd8ecc8c0901d482b601446a4f4f3e02 100644 (file)
@@ -43,6 +43,8 @@
 #include <cstring>
 
 #include <algorithm>
+#include <string>
+#include <vector>
 
 #include "gromacs/gmxpreprocess/fflibutil.h"
 #include "gromacs/gmxpreprocess/notset.h"
 
 gpp_atomtype_t read_atype(const char *ffdir, t_symtab *tab)
 {
-    int            nfile, f;
-    char         **file;
-    FILE          *in;
-    char           buf[STRLEN], name[STRLEN];
-    double         m;
-    int            nratt = 0;
-    gpp_atomtype_t at;
-    t_atom        *a;
-    t_param       *nb;
-
-    nfile = fflib_search_file_end(ffdir, ".atp", TRUE, &file);
+    FILE                    *in;
+    char                     buf[STRLEN], name[STRLEN];
+    double                   m;
+    int                      nratt = 0;
+    gpp_atomtype_t           at;
+    t_atom                  *a;
+    t_param                 *nb;
+
+    std::vector<std::string> files = fflib_search_file_end(ffdir, ".atp", TRUE);
     at    = init_atomtype();
     snew(a, 1);
     snew(nb, 1);
 
-    for (f = 0; f < nfile; f++)
+    for (const auto &filename : files)
     {
-        in = fflib_open(file[f]);
+        in = fflib_open(filename);
         while (!feof(in))
         {
             /* Skip blank or comment-only lines */
@@ -100,10 +100,8 @@ gpp_atomtype_t read_atype(const char *ffdir, t_symtab *tab)
             }
         }
         gmx_ffclose(in);
-        sfree(file[f]);
     }
     fprintf(stderr, "\n");
-    sfree(file);
 
     return at;
 }
@@ -245,7 +243,7 @@ static void print_resbondeds(FILE *out, int bt, t_restp *rtp)
     }
 }
 
-static void check_rtp(int nrtp, t_restp rtp[], char *libfn)
+static void check_rtp(int nrtp, t_restp rtp[], const char *libfn)
 {
     int i;
 
@@ -320,7 +318,7 @@ void print_resall(FILE *out, int nrtp, t_restp rtp[],
     }
 }
 
-void read_resall(char *rrdb, int *nrtpptr, t_restp **rtp,
+void read_resall(const char *rrdb, int *nrtpptr, t_restp **rtp,
                  gpp_atomtype_t atype, t_symtab *tab,
                  bool bAllowOverrideRTP)
 {
index b72e630698e48b2808d136a0d4d72411e54aa472..df7bc35f91e1fef859a627c7b0ba9eb97679c73a 100644 (file)
@@ -56,7 +56,7 @@ t_restp *get_restp(const char *rtpname, int nrtp, t_restp rtp[]);
 gpp_atomtype_t read_atype(const char *ffdir, struct t_symtab *tab);
 /* read atom type database(s) */
 
-void read_resall(char *resdb, int *nrtp, t_restp **rtp,
+void read_resall(const char *resdb, int *nrtp, t_restp **rtp,
                  gpp_atomtype_t atype, struct t_symtab *tab,
                  bool bAllowOverrideRTP);
 /* read rtp database, append to the existing database */
index 782ad1be9ddc6734627e4060f96f621c5e680bed..eb2f80a06697fde2047234b2fa91658088b65664 100644 (file)
@@ -41,6 +41,9 @@
 #include <cctype>
 #include <cstring>
 
+#include <string>
+#include <vector>
+
 #include "gromacs/fileio/gmxfio.h"
 #include "gromacs/gmxpreprocess/fflibutil.h"
 #include "gromacs/gmxpreprocess/h_db.h"
@@ -242,7 +245,7 @@ static void print_ter_db(const char *ff, char C, int nb, t_hackblock tb[],
     gmx_fio_fclose(out);
 }
 
-static void read_ter_db_file(char *fn,
+static void read_ter_db_file(const char *fn,
                              int *ntbptr, t_hackblock **tbptr,
                              gpp_atomtype_t atype)
 {
@@ -403,8 +406,6 @@ int read_ter_db(const char *ffdir, char ter,
                 t_hackblock **tbptr, gpp_atomtype_t atype)
 {
     char   ext[STRLEN];
-    int    ntdbf, f;
-    char **tdbf;
     int    ntb;
 
     sprintf(ext, ".%c.tdb", ter);
@@ -412,15 +413,13 @@ int read_ter_db(const char *ffdir, char ter,
     /* Search for termini database files.
      * Do not generate an error when none are found.
      */
-    ntdbf  = fflib_search_file_end(ffdir, ext, FALSE, &tdbf);
+    std::vector<std::string> tdbf  = fflib_search_file_end(ffdir, ext, FALSE);
     ntb    = 0;
     *tbptr = nullptr;
-    for (f = 0; f < ntdbf; f++)
+    for (const auto &filename : tdbf)
     {
-        read_ter_db_file(tdbf[f], &ntb, tbptr, atype);
-        sfree(tdbf[f]);
+        read_ter_db_file(filename.c_str(), &ntb, tbptr, atype);
     }
-    sfree(tdbf);
 
     if (debug)
     {
index 60d4d8e54debd80785bc6a782d950420c21380e7..59191820305958556acbfdcabd3e10dab6e4c124 100644 (file)
@@ -41,6 +41,9 @@
 #include <cctype>
 #include <cstring>
 
+#include <string>
+#include <vector>
+
 #include "gromacs/gmxpreprocess/fflibutil.h"
 #include "gromacs/gmxpreprocess/hackblock.h"
 #include "gromacs/topology/residuetypes.h"
@@ -58,7 +61,7 @@ typedef struct {
     char *replace;
 } t_xlate_atom;
 
-static void get_xlatoms(const char *fn, FILE *fp,
+static void get_xlatoms(const std::string &filename, FILE *fp,
                         int *nptr, t_xlate_atom **xlptr)
 {
     char          filebase[STRLEN];
@@ -68,7 +71,7 @@ static void get_xlatoms(const char *fn, FILE *fp,
     int           n, na, idum;
     t_xlate_atom *xl;
 
-    fflib_filename_base(fn, filebase, STRLEN);
+    fflib_filename_base(filename.c_str(), filebase, STRLEN);
 
     n  = *nptr;
     xl = *xlptr;
@@ -85,7 +88,7 @@ static void get_xlatoms(const char *fn, FILE *fp,
         }
         if (na != 3)
         {
-            gmx_fatal(FARGS, "Expected a residue name and two atom names in file '%s', not '%s'", fn, line);
+            gmx_fatal(FARGS, "Expected a residue name and two atom names in file '%s', not '%s'", filename.c_str(), line);
         }
 
         srenew(xl, n+1);
@@ -133,7 +136,7 @@ static void done_xlatom(int nxlate, t_xlate_atom *xlatom)
     sfree(xlatom);
 }
 
-void rename_atoms(const char *xlfile, const char *ffdir,
+void rename_atoms(const charxlfile, const char *ffdir,
                   t_atoms *atoms, t_symtab *symtab, const t_restp *restp,
                   bool bResname, gmx_residuetype_t *rt, bool bReorderNum,
                   bool bVerbose)
@@ -141,8 +144,6 @@ void rename_atoms(const char *xlfile, const char *ffdir,
     FILE         *fp;
     int           nxlate, a, i, resind;
     t_xlate_atom *xlatom;
-    int           nf;
-    char        **f;
     char          c, *rnm, atombuf[32], *ptr0, *ptr1;
     bool          bReorderedNum, bRenamed, bMatch;
     bool          bStartTerm, bEndTerm;
@@ -157,15 +158,13 @@ void rename_atoms(const char *xlfile, const char *ffdir,
     }
     else
     {
-        nf = fflib_search_file_end(ffdir, ".arn", FALSE, &f);
-        for (i = 0; i < nf; i++)
+        std::vector<std::string> fns = fflib_search_file_end(ffdir, ".arn", FALSE);
+        for (const auto &filename : fns)
         {
-            fp = fflib_open(f[i]);
-            get_xlatoms(f[i], fp, &nxlate, &xlatom);
+            fp = fflib_open(filename);
+            get_xlatoms(filename, fp, &nxlate, &xlatom);
             gmx_ffclose(fp);
-            sfree(f[i]);
         }
-        sfree(f);
     }
 
     for (a = 0; (a < atoms->nr); a++)
index 1b8da84fdfeede513fd33496fa70f603db2c0bf1..f11c17caee9537712f36de6f9dc57be224e12033 100644 (file)
@@ -48,7 +48,7 @@ struct t_symtab;
 /* If bResname is true renames atoms based on residue names,
  * otherwise renames atoms based on rtp entry names.
  */
-void rename_atoms(const char *xlfile, const char *ffdir,
+void rename_atoms(const charxlfile, const char *ffdir,
                   struct t_atoms *atoms, struct t_symtab *symtab, const t_restp *restp,
                   bool bResname, struct gmx_residuetype_t *rt, bool bReorderNum,
                   bool bVerbose);