Fix window::size() semantics, standardize comm_ naming, simplify tests

Address remaining inconsistencies and improve code clarity:

- Standardize communicator member variable naming from com_ to comm_ across all classes for consistency with mpi::window

- Fix window::size() semantics to return element count instead of bytes, matching constructor parameter units

- Simplify test code in mpi_window.cpp:
  * Use structured bindings and modern C++ patterns
  * Remove redundant variables and computations
  * Clearer comments and better use of mpi::chunk

- Fix window::owned_ member initialization ordering to correctly handle both MPI and non-MPI cases

- Remove redundant forward declaration of shared_window

- Simplify get_communicator() implementations to return comm_ directly

- Update build_multi_node.yml workflow with improvements from app4triqs

- Minor documentation clarifications in group and window classes

Author: Wentzell

.github/workflows/build_multi_node.yml

@@ -1,10 +1,10 @@
-name: build
+name: build_multi_node
 
 on:
   push:
-    branches: [ unstable ]
+    branches: [ unstable, '[0-9]+.[0-9]+.x' ]
   pull_request:
-    branches: [ unstable ]
+    branches: [ unstable, '[0-9]+.[0-9]+.x' ]
   workflow_call:
   workflow_dispatch:
 
@@ -33,8 +33,8 @@ jobs:
     - uses: actions/cache/restore@v4
       with:
         path: ${{ env.CCACHE_DIR }}
-        key: ccache-${{ matrix.os }}-${{ matrix.cc }}-${{ github.run_id }}
-        restore-keys:
+        key: ccache-${{ matrix.os }}-${{ matrix.cc }}-${{ github.run_id }}-${{ github.run_attempt }}
+        restore-keys: |
           ccache-${{ matrix.os }}-${{ matrix.cc }}-
 
     - name: Install ubuntu dependencies
@@ -61,7 +61,7 @@ jobs:
         cmake --build build/ --target test
         '
 
-    - name: Run tests inside the container
+    - name: Run multi-node window tests
       run: |
         docker exec -t -u runner -w ${{ github.workspace }} docker-vm-1 /bin/bash -euxc '
         cat <<EOF | tee hostfile
@@ -85,4 +85,4 @@ jobs:
       if: always()
       with:
         path: ${{ env.CCACHE_DIR }}
-        key: ccache-${{ matrix.os }}-${{ matrix.cc }}-${{ github.run_id }}
+        key: ccache-${{ matrix.os }}-${{ matrix.cc }}-${{ github.run_id }}-${{ github.run_attempt }}

c++/mpi/communicator.hpp

@@ -55,21 +55,21 @@ namespace mpi {
      * @details The `MPI_Comm` object is copied without calling `MPI_Comm_dup`.
      * @param c `MPI_Comm` object to wrap.
      */
-    communicator(MPI_Comm c) : com_(c) {}
+    communicator(MPI_Comm c) : comm_(c) {}
 
     /// Get the wrapped `MPI_Comm` object.
-    [[nodiscard]] MPI_Comm get() const noexcept { return com_; }
+    [[nodiscard]] MPI_Comm get() const noexcept { return comm_; }
 
     /// Check if the contained `MPI_Comm` is `MPI_COMM_NULL`.
-    [[nodiscard]] bool is_null() const noexcept { return com_ == MPI_COMM_NULL; }
+    [[nodiscard]] bool is_null() const noexcept { return comm_ == MPI_COMM_NULL; }
 
     /**
      * @brief Get the rank of the calling process in the communicator.
      * @return The result of `MPI_Comm_rank` if mpi::has_env is true, otherwise 0.
      */
     [[nodiscard]] int rank() const {
       int r = 0;
-      if (has_env) check_mpi_call(MPI_Comm_rank(com_, &r), "MPI_Comm_rank");
+      if (has_env) check_mpi_call(MPI_Comm_rank(comm_, &r), "MPI_Comm_rank");
       return r;
     }
 
@@ -79,7 +79,7 @@ namespace mpi {
      */
     [[nodiscard]] int size() const {
       int s = 1;
-      if (has_env) check_mpi_call(MPI_Comm_size(com_, &s), "MPI_Comm_size");
+      if (has_env) check_mpi_call(MPI_Comm_size(comm_, &s), "MPI_Comm_size");
       return s;
     }
 
@@ -100,7 +100,7 @@ namespace mpi {
      */
     [[nodiscard]] communicator split(int color, int key = 0) const {
       communicator c{};
-      if (has_env) check_mpi_call(MPI_Comm_split(com_, color, key, &c.com_), "MPI_Comm_split");
+      if (has_env) check_mpi_call(MPI_Comm_split(comm_, color, key, &c.comm_), "MPI_Comm_split");
       return c;
     }
 
@@ -134,7 +134,7 @@ namespace mpi {
      */
     [[nodiscard]] communicator duplicate() const {
       communicator c{};
-      if (has_env) check_mpi_call(MPI_Comm_dup(com_, &c.com_), "MPI_Comm_dup");
+      if (has_env) check_mpi_call(MPI_Comm_dup(comm_, &c.comm_), "MPI_Comm_dup");
       return c;
     }
 
@@ -148,7 +148,7 @@ namespace mpi {
      * Does nothing, if mpi::has_env is false.
      */
     void free() {
-      if (has_env && !is_null()) check_mpi_call(MPI_Comm_free(&com_), "MPI_Comm_free");
+      if (has_env && !is_null()) check_mpi_call(MPI_Comm_free(&comm_), "MPI_Comm_free");
     }
 
     /**
@@ -157,16 +157,16 @@ namespace mpi {
      */
     void abort(int error_code) const {
       if (has_env) {
-        check_mpi_call(MPI_Abort(com_, error_code), "MPI_Abort");
+        check_mpi_call(MPI_Abort(comm_, error_code), "MPI_Abort");
       } else {
         std::abort();
       }
     }
 
 #ifdef BOOST_MPI_HPP
     // Conversion to and from boost communicator, Keep for backward compatibility
-    inline operator boost::mpi::communicator() const { return boost::mpi::communicator(_com, boost::mpi::comm_duplicate); }
-    inline communicator(boost::mpi::communicator c) : _com(c) {}
+    inline operator boost::mpi::communicator() const { return boost::mpi::communicator(comm_, boost::mpi::comm_duplicate); }
+    inline communicator(boost::mpi::communicator c) : comm_(c) {}
 #endif // BOOST_MPI_HPP
 
     /**
@@ -188,11 +188,11 @@ namespace mpi {
     void barrier(long poll_msec = 1) const {
       if (has_env) {
         if (poll_msec == 0) {
-          check_mpi_call(MPI_Barrier(com_), "MPI_Barrier");
+          check_mpi_call(MPI_Barrier(comm_), "MPI_Barrier");
         } else {
           MPI_Request req{};
           int flag = 0;
-          check_mpi_call(MPI_Ibarrier(com_, &req), "MPI_Ibarrier");
+          check_mpi_call(MPI_Ibarrier(comm_, &req), "MPI_Ibarrier");
           while (!flag) {
             check_mpi_call(MPI_Test(&req, &flag, MPI_STATUS_IGNORE), "MPI_Test");
             usleep(poll_msec * 1000);
@@ -202,7 +202,7 @@ namespace mpi {
     }
 
     private:
-    MPI_Comm com_ = MPI_COMM_WORLD;
+    MPI_Comm comm_ = MPI_COMM_WORLD;
   };
 
   /**
@@ -224,7 +224,7 @@ namespace mpi {
 
   [[nodiscard]] inline shared_communicator communicator::split_shared(int split_type, int key) const {
     shared_communicator c{};
-    if (has_env) check_mpi_call(MPI_Comm_split_type(com_, split_type, key, MPI_INFO_NULL, &c.com_), "MPI_Comm_split_type");
+    if (has_env) check_mpi_call(MPI_Comm_split_type(comm_, split_type, key, MPI_INFO_NULL, &c.comm_), "MPI_Comm_split_type");
     return c;
   }
 

c++/mpi/group.hpp

@@ -36,8 +36,10 @@ namespace mpi {
    * @ingroup mpi_essentials
    * @brief C++ wrapper around `MPI_Group` providing various convenience functions.
    *
-   * @details It stores an `MPI_Group` object as its only member which by default is set to `MPI_GROUP_NULL`. The
-   * underlying `MPI_Group` object is automatically freed when a group object goes out of scope.
+   * @details It stores an `MPI_Group` object as its only member which by default is set to `MPI_GROUP_NULL`.
+   * The underlying `MPI_Group` object is automatically freed when a group object goes out of scope.
+   *
+   * This class follows move-only semantics and takes ownership of the wrapped `MPI_Group` object.
    *
    * All functions that make direct calls to the MPI C library throw an exception in case the call fails.
    */
@@ -98,8 +100,8 @@ namespace mpi {
     }
 
     /**
-     * @brief Get the size of the communicator.
-     * @return The result of `MPI_Comm_size` if mpi::has_env is true, otherwise 1.
+     * @brief Get the size of the group.
+     * @return The result of `MPI_Group_size` if mpi::has_env is true, otherwise 1.
      */
     [[nodiscard]] int size() const {
       int s = 1;

c++/mpi/window.hpp

@@ -41,9 +41,6 @@ namespace mpi {
    * @{
    */
 
-  // Forward declaration.
-  template <class BaseType> class shared_window;
-
   /**
    * @brief A C++ wrapper around `MPI_Win` providing convenient memory window management.
    *
@@ -53,6 +50,8 @@ namespace mpi {
    *
    * If a base pointer is not specified, the constructor will allocate memory internally.
    *
+   * This class follows move-only semantics and takes ownership of the wrapped `MPI_Win` object.
+   *
    * @tparam BaseType The type of elements stored in the memory window.
    */
   template <class BaseType> class window {
@@ -125,9 +124,9 @@ namespace mpi {
       if (has_env) {
         check_mpi_call(MPI_Win_allocate(size_ * sizeof(BaseType), sizeof(BaseType), info, c.get(), &data_, &win_), "MPI_Win_allocate");
       } else {
-        owned_ = true;
         data_  = new BaseType[size_]; // NOLINT (new is fine here)
       }
+      owned_ = true;
     }
 
     /// Convert the window to the wrapped `MPI_Win` object.
@@ -143,7 +142,7 @@ namespace mpi {
      * @brief Release allocated resources owned by the window.
      *
      * @details Before freeing the owned memory or the `MPI_Win` handle, a window must have completed all its
-     * involvement in RMA communications. For that reason we call fence() before `MPI_Win_free`.
+     * involvement in RMA communications. For that reason we call `MPI_Win_fence` before `MPI_Win_free`.
      *
      * The window also must be unlocked if it has been previously locked. However, this cannot be detected and is
      * therefore the responsibility of the user.
@@ -154,7 +153,7 @@ namespace mpi {
     void free() noexcept {
       if (has_env) {
         if (win_ != MPI_WIN_NULL) {
-          fence();
+          MPI_Win_fence(0, win_);
           MPI_Win_free(&win_);
         }
       } else if (owned_) {
@@ -338,14 +337,14 @@ namespace mpi {
     /// Get a pointer to the beginning of the window memory.
     [[nodiscard]] BaseType *base() const { return data_; }
 
-    /// Get the size of the window in bytes.
-    [[nodiscard]] MPI_Aint size() const { return size_ * sizeof(BaseType); }
+    /// Get the size of the window in number of elements.
+    [[nodiscard]] MPI_Aint size() const { return size_; }
 
     /// Get the displacement unit in bytes.
     [[nodiscard]] int disp_unit() const { return sizeof(BaseType); }
 
     /// Get the mpi::communicator associated with the window.
-    [[nodiscard]] communicator get_communicator() const { return comm_.get(); }
+    [[nodiscard]] communicator get_communicator() const { return comm_; }
 
     protected:
     MPI_Win win_{MPI_WIN_NULL};
@@ -364,7 +363,7 @@ namespace mpi {
    */
   template <class BaseType> class shared_window : public window<BaseType> {
     public:
-    ///Construct a shared memory window with `MPI_WIN_NULL`.
+    /// Construct a shared memory window with `MPI_WIN_NULL`.
     shared_window() = default;
 
     /**
@@ -384,19 +383,22 @@ namespace mpi {
       if (has_env) {
         check_mpi_call(MPI_Win_allocate_shared(size_ * sizeof(BaseType), sizeof(BaseType), info, c.get(), &data_, &win_), "MPI_Win_allocate_shared");
       } else {
-        owned_ = true;
         data_  = new BaseType[size_]; // NOLINT (new is fine here)
       }
+      owned_ = true;
     }
 
     /**
      * @brief Query attributes of a shared memory window.
      *
-     * @details Retrieves the size, displacement unit, and a pointer to the beginning of the shared memory region for a
+     * @details Retrieves the byte-size, displacement unit, and a pointer to the beginning of the shared memory region for a
      * specific rank.
      *
+     * When `MPI_PROC_NULL` is passed for the rank, MPI returns information about the memory segment with the lowest rank that has a
+     * non-zero size.
+     *
      * @param rank Rank within the shared communicator.
-     * @return A tuple containing the size in bytes, the displacement unit in bytes and the base pointer.
+     * @return A tuple containing the byte-size, the displacement unit in bytes and the base pointer.
      */
     [[nodiscard]] std::tuple<MPI_Aint, int, void *> query(int rank = MPI_PROC_NULL) const {
       if (has_env) {