From c10695c5d5756ce8fbda3bc7f49cbd577a9a47c4 Mon Sep 17 00:00:00 2001
From: Vegard Kippe <vkip@equinor.com>
Date: Tue, 9 Jul 2024 13:48:04 +0200
Subject: [PATCH] Switch to using size_t instead of int for buffer position,
 and properly account for MPI using int

---
 opm/simulators/utils/MPIPacker.cpp            | 28 +++++-----
 opm/simulators/utils/MPIPacker.hpp            | 52 ++++++++++++-------
 opm/simulators/utils/SerializationPackers.cpp |  4 +-
 opm/simulators/utils/SerializationPackers.hpp |  4 +-
 4 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/opm/simulators/utils/MPIPacker.cpp b/opm/simulators/utils/MPIPacker.cpp
index 5d0d08ce1..601f53f37 100644
--- a/opm/simulators/utils/MPIPacker.cpp
+++ b/opm/simulators/utils/MPIPacker.cpp
@@ -45,7 +45,7 @@ template<std::size_t Size>
 void Packing<false,std::bitset<Size>>::
 pack(const std::bitset<Size>& data,
      std::vector<char>& buffer,
-     int& position,
+     std::size_t& position,
      Parallel::MPIComm comm)
 {
     Packing<true,unsigned long long>::pack(data.to_ullong(), buffer, position, comm);
@@ -55,7 +55,7 @@ template<std::size_t Size>
 void Packing<false,std::bitset<Size>>::
 unpack(std::bitset<Size>& data,
        std::vector<char>& buffer,
-       int& position,
+       std::size_t& position,
        Parallel::MPIComm comm)
 {
     unsigned long long d;
@@ -76,28 +76,32 @@ packSize(const std::string& data, Parallel::MPIComm comm)
 void Packing<false,std::string>::
 pack(const std::string& data,
      std::vector<char>& buffer,
-     int& position,
+     std::size_t& position,
      Parallel::MPIComm comm)
 {
     std::size_t length = data.size();
-    MPI_Pack(&length, 1, Dune::MPITraits<std::size_t>::getType(), buffer.data(),
-             buffer.size(), &position, comm);
-    MPI_Pack(data.data(), length, MPI_CHAR, buffer.data(), buffer.size(),
-             &position, comm);
+    int int_position;
+    MPI_Pack(&length, 1, Dune::MPITraits<std::size_t>::getType(), buffer.data()+position,
+             mpi_buffer_size(buffer.size(), position), &int_position, comm);
+    MPI_Pack(data.data(), length, MPI_CHAR, buffer.data()+position, mpi_buffer_size(buffer.size(), position),
+             &int_position, comm);
+    position += int_position;
 }
 
 void Packing<false,std::string>::
 unpack(std::string& data,
        std::vector<char>& buffer,
-       int& position,
+       std::size_t& position,
        Opm::Parallel::MPIComm comm)
 {
     std::size_t length = 0;
-    MPI_Unpack(buffer.data(), buffer.size(), &position, &length, 1,
+    int int_position;
+    MPI_Unpack(buffer.data()+position, mpi_buffer_size(buffer.size(), position), &int_position, &length, 1,
                Dune::MPITraits<std::size_t>::getType(), comm);
     std::vector<char> cStr(length+1, '\0');
-    MPI_Unpack(buffer.data(), buffer.size(), &position, cStr.data(), length,
+    MPI_Unpack(buffer.data()+position, mpi_buffer_size(buffer.size(), position), &int_position, cStr.data(), length,
                MPI_CHAR, comm);
+    position += int_position;
     data.clear();
     data.append(cStr.data(), length);
 }
@@ -111,7 +115,7 @@ packSize(const time_point&, Opm::Parallel::MPIComm comm)
 void Packing<false,time_point>::
 pack(const time_point& data,
      std::vector<char>& buffer,
-     int& position,
+     std::size_t& position,
      Parallel::MPIComm comm)
 {
     Packing<true,std::time_t>::pack(TimeService::to_time_t(data),
@@ -121,7 +125,7 @@ pack(const time_point& data,
 void Packing<false,time_point>::
 unpack(time_point& data,
        std::vector<char>& buffer,
-       int& position,
+       std::size_t& position,
        Parallel::MPIComm comm)
 {
     std::time_t res;
diff --git a/opm/simulators/utils/MPIPacker.hpp b/opm/simulators/utils/MPIPacker.hpp
index 9d5173e8c..056fc1255 100644
--- a/opm/simulators/utils/MPIPacker.hpp
+++ b/opm/simulators/utils/MPIPacker.hpp
@@ -26,19 +26,26 @@
 
 #include <bitset>
 #include <cstddef>
+#include <limits>
 #include <string>
 
+
 namespace Opm {
 namespace Mpi {
 namespace detail {
 
+static std::size_t mpi_buffer_size(const std::size_t bufsize, const std::size_t position) {
+    return static_cast<int>(std::min(bufsize-position,
+                                     static_cast<std::size_t>(std::numeric_limits<int>::max())));
+}
+
 //! \brief Abstract struct for packing which is (partially) specialized for specific types.
 template <bool pod, class T>
 struct Packing
 {
     static std::size_t packSize(const T&, Parallel::MPIComm);
-    static void pack(const T&, std::vector<char>&, int&, Parallel::MPIComm);
-    static void unpack(T&, std::vector<char>&, int&, Parallel::MPIComm);
+    static void pack(const T&, std::vector<char>&, std::size_t&, Parallel::MPIComm);
+    static void unpack(T&, std::vector<char>&, std::size_t&, Parallel::MPIComm);
 };
 
 //! \brief Packaging for pod data.
@@ -59,6 +66,9 @@ struct Packing<true,T>
     //! \param comm The communicator to use
     static std::size_t packSize(const T*, std::size_t n, Parallel::MPIComm comm)
     {
+        // For now we do not handle the situation where a a single call to packSize/pack/unpack
+        // is likely to require an MPI_Pack_size value larger than intmax
+        assert ( n*sizeof(T) <= std::numeric_limits<int>::max() );
         int size = 0;
         MPI_Pack_size(n, Dune::MPITraits<T>::getType(), comm, &size);
         return size;
@@ -71,7 +81,7 @@ struct Packing<true,T>
     //! \param comm The communicator to use
     static void pack(const T& data,
                      std::vector<char>& buffer,
-                     int& position,
+                     std::size_t& position,
                      Parallel::MPIComm comm)
     {
         pack(&data, 1, buffer, position, comm);
@@ -86,11 +96,13 @@ struct Packing<true,T>
     static void pack(const T* data,
                      std::size_t n,
                      std::vector<char>& buffer,
-                     int& position,
+                     std::size_t& position,
                      Parallel::MPIComm comm)
     {
-        MPI_Pack(data, n, Dune::MPITraits<T>::getType(), buffer.data(),
-                 buffer.size(), &position, comm);
+        int int_position = 0;
+        MPI_Pack(data, n, Dune::MPITraits<T>::getType(), buffer.data()+position,
+                 mpi_buffer_size(buffer.size(), position), &int_position, comm);
+        position += int_position;
     }
 
     //! \brief Unpack a POD.
@@ -100,7 +112,7 @@ struct Packing<true,T>
     //! \param comm The communicator to use
     static void unpack(T& data,
                        std::vector<char>& buffer,
-                       int& position,
+                       std::size_t& position,
                        Parallel::MPIComm comm)
     {
         unpack(&data, 1, buffer, position, comm);
@@ -115,11 +127,13 @@ struct Packing<true,T>
     static void unpack(T* data,
                        std::size_t n,
                        std::vector<char>& buffer,
-                       int& position,
+                       std::size_t& position,
                        Parallel::MPIComm comm)
     {
-        MPI_Unpack(buffer.data(), buffer.size(), &position, data, n,
+        int int_position = 0;
+        MPI_Unpack(buffer.data()+position, mpi_buffer_size(buffer.size(), position), &int_position, data, n,
                    Dune::MPITraits<T>::getType(), comm);
+        position += int_position;
     }
 };
 
@@ -133,13 +147,13 @@ struct Packing<false,T>
         return 0;
     }
 
-    static void pack(const T&, std::vector<char>&, int&,
+    static void pack(const T&, std::vector<char>&, std::size_t&,
                      Parallel::MPIComm)
     {
       static_assert(!std::is_same_v<T,T>, "Packing not supported for type");
     }
 
-    static void unpack(T&, std::vector<char>&, int&,
+    static void unpack(T&, std::vector<char>&, std::size_t&,
                        Parallel::MPIComm)
     {
         static_assert(!std::is_same_v<T,T>, "Packing not supported for type");
@@ -151,8 +165,8 @@ template <std::size_t Size>
 struct Packing<false,std::bitset<Size>>
 {
     static std::size_t packSize(const std::bitset<Size>&, Opm::Parallel::MPIComm);
-    static void pack(const std::bitset<Size>&, std::vector<char>&, int&, Opm::Parallel::MPIComm);
-    static void unpack(std::bitset<Size>&, std::vector<char>&, int&, Opm::Parallel::MPIComm);
+    static void pack(const std::bitset<Size>&, std::vector<char>&, std::size_t&, Opm::Parallel::MPIComm);
+    static void unpack(std::bitset<Size>&, std::vector<char>&, std::size_t&, Opm::Parallel::MPIComm);
 };
 
 #define ADD_PACK_SPECIALIZATION(T) \
@@ -160,8 +174,8 @@ struct Packing<false,std::bitset<Size>>
     struct Packing<false,T> \
     { \
         static std::size_t packSize(const T&, Parallel::MPIComm); \
-        static void pack(const T&, std::vector<char>&, int&, Parallel::MPIComm); \
-        static void unpack(T&, std::vector<char>&, int&, Parallel::MPIComm); \
+        static void pack(const T&, std::vector<char>&, std::size_t&, Parallel::MPIComm); \
+        static void unpack(T&, std::vector<char>&, std::size_t&, Parallel::MPIComm); \
     };
 
 ADD_PACK_SPECIALIZATION(std::string)
@@ -207,7 +221,7 @@ struct Packer {
     template<class T>
     void pack(const T& data,
               std::vector<char>& buffer,
-              int& position) const
+              std::size_t& position) const
     {
         detail::Packing<std::is_pod_v<T>,T>::pack(data, buffer, position, m_comm);
     }
@@ -222,7 +236,7 @@ struct Packer {
     void pack(const T* data,
               std::size_t n,
               std::vector<char>& buffer,
-              int& position) const
+              std::size_t& position) const
     {
         static_assert(std::is_pod_v<T>, "Array packing not supported for non-pod data");
         detail::Packing<true,T>::pack(data, n, buffer, position, m_comm);
@@ -236,7 +250,7 @@ struct Packer {
     template<class T>
     void unpack(T& data,
                 std::vector<char>& buffer,
-                int& position) const
+                std::size_t& position) const
     {
         detail::Packing<std::is_pod_v<T>,T>::unpack(data, buffer, position, m_comm);
     }
@@ -251,7 +265,7 @@ struct Packer {
     void unpack(T* data,
                 std::size_t n,
                 std::vector<char>& buffer,
-                int& position) const
+                std::size_t& position) const
     {
         static_assert(std::is_pod_v<T>, "Array packing not supported for non-pod data");
         detail::Packing<true,T>::unpack(data, n, buffer, position, m_comm);
diff --git a/opm/simulators/utils/SerializationPackers.cpp b/opm/simulators/utils/SerializationPackers.cpp
index 6d7d927db..4c5879351 100644
--- a/opm/simulators/utils/SerializationPackers.cpp
+++ b/opm/simulators/utils/SerializationPackers.cpp
@@ -34,14 +34,14 @@ packSize(const boost::gregorian::date& data)
 
 void Packing<false,boost::gregorian::date>::
 pack(const boost::gregorian::date& data,
-     std::vector<char>& buffer, int& position)
+     std::vector<char>& buffer, std::size_t& position)
 {
     Packing<false,std::string>::pack(boost::gregorian::to_simple_string(data), buffer, position);
 }
 
 void Packing<false,boost::gregorian::date>::
 unpack(boost::gregorian::date& data,
-       std::vector<char>& buffer, int& position)
+       std::vector<char>& buffer, std::size_t& position)
 {
     std::string date;
     Packing<false,std::string>::unpack(date, buffer, position);
diff --git a/opm/simulators/utils/SerializationPackers.hpp b/opm/simulators/utils/SerializationPackers.hpp
index d30bb50df..9b3a17cd2 100644
--- a/opm/simulators/utils/SerializationPackers.hpp
+++ b/opm/simulators/utils/SerializationPackers.hpp
@@ -35,10 +35,10 @@ struct Packing<false,boost::gregorian::date>
     static std::size_t packSize(const boost::gregorian::date& data);
 
     static void pack(const boost::gregorian::date& data,
-                     std::vector<char>& buffer, int& position);
+                     std::vector<char>& buffer, std::size_t& position);
 
     static void unpack(boost::gregorian::date& data,
-                       std::vector<char>& buffer, int& position);
+                       std::vector<char>& buffer, std::size_t& position);
 };
 
 }