Add review suggestions

include/openPMD/ChunkInfo.hpp

@@ -27,6 +27,9 @@
 #if openPMD_HAVE_MPI
 #include <mpi.h>
 #endif
+
+#include <map>
+#include <string>
 #include <vector>
 
 namespace openPMD
@@ -77,7 +80,6 @@ struct WrittenChunkInfo : ChunkInfo
     bool operator==(WrittenChunkInfo const &other) const;
 };
 
-// !< @todo Also add a ChunkTable for ReadChunkInfo or sth like that
 using ChunkTable = std::vector<WrittenChunkInfo>;
 
 namespace chunk_assignment

include/openPMD/Series.hpp

@@ -44,6 +44,7 @@
 #include <optional>
 #include <set>
 #include <string>
+#include <variant>
 
 // expose private and protected members for invasive testing
 #ifndef OPENPMD_private
@@ -215,7 +216,6 @@ namespace internal
         struct RankTableData
         {
             Attributable m_attributable;
-            // Parameter<Operation::CREATE_DATASET> m_param;
             std::variant<
                 NoSourceSpecified,
                 SourceSpecifiedViaJSON,

include/openPMD/auxiliary/Mpi.hpp

@@ -67,16 +67,50 @@ namespace
     }
 } // namespace
 
+/**
+ * Multiple variable-length strings represented in one single buffer
+ * with a fixed line width.
+ * Strings smaller than the maximum width are padded with zeros.
+ * The length of char_buffer should be equal to the product of line_length
+ * and num_lines.
+ */
 struct StringMatrix
 {
     std::vector<char> char_buffer;
     size_t line_length = 0;
     size_t num_lines = 0;
 };
 
+/*
+ * These are mostly internal helper functions, so this defines only those that
+ * we need.
+ * Logically, these should be complemented by `collectStringsTo()` and
+ * `distributeStringsAsMatrixToAllRanks()`, but we don't need them (yet).
+ */
+
+/**
+ * @brief Collect multiple variable-length strings to one rank in MPI_Gatherv
+ *        fashion. Uses two collective MPI calls, the first to gather the
+ *        different string lengths, the second to gather the actual strings.
+ *
+ * @param communicator MPI communicator
+ * @param destRank Target rank for MPI_Gatherv
+ * @param thisRankString The current MPI rank's contribution to the data.
+ * @return StringMatrix See documentation of StringMatrix struct.
+ */
 StringMatrix collectStringsAsMatrixTo(
     MPI_Comm communicator, int destRank, std::string const &thisRankString);
 
+/**
+ * @brief Collect multiple variable-length strings to all ranks in
+ *        MPI_Allgatherv fashion. Uses two collective MPI calls, the first to
+ *        gather the different string lengths, the second to gather the actual
+ *        strings.
+ *
+ * @param communicator communicator
+ * @param thisRankString The current MPI rank's contribution to the data.
+ * @return std::vector<std::string> All ranks' strings, returned on all ranks.
+ */
 std::vector<std::string> distributeStringsToAllRanks(
     MPI_Comm communicator, std::string const &thisRankString);
 #endif

include/openPMD/binding/python/Mpi.hpp

@@ -25,9 +25,9 @@
 
 #if openPMD_HAVE_MPI
 
+#include "openPMD/binding/python/Common.hpp"
+
 #include <mpi.h>
-#include <pybind11/pybind11.h>
-#include <pybind11/stl.h>
 
 /** mpi4py communicator wrapper
  *

src/auxiliary/Mpi.cpp

@@ -41,7 +41,7 @@ StringMatrix collectStringsAsMatrixTo(
     {
         res.line_length = maxLength;
         res.num_lines = size;
-        res.char_buffer.resize(res.line_length * res.num_lines);
+        res.char_buffer.resize(maxLength * res.num_lines);
         displs.reserve(size);
         for (int i = 0; i < size; ++i)
         {