From dbd595487ba7b988ed5100760e6269d6b23ee838 Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Mon, 7 Nov 2022 17:34:38 +0100
Subject: [PATCH] ENH: preserve globalIndex merge information within mergedSurf

- for later reuse with fields (for example)

ENH: use 'scheduled' for surfaceWriter field merging (#2402)

- in tests with merging fields (surfaceWriter), 'scheduled' was
  generally faster than 'nonBlocking' for scalars, minorly faster for
  vectors.
  Thus make 'scheduled' the default for the surfaceWriter but with a
  user-option to adjust as required. Previously simply relied on
  whichever default globalIndex had (currently nonBlocking).

  Reuse globalIndex information from mergedSurf instead of
  globalIndex::gatherOp to avoid an extra MPI call to gather sizes
  each time.

  These changes will not be noticable unless surface sampling is done
  very frequently (eg, every iteration) and with large core counts.
---
 src/surfMesh/mergedSurf/mergedSurf.C          | 41 ++++-----
 src/surfMesh/mergedSurf/mergedSurf.H          | 83 ++++++++++++-------
 src/surfMesh/meshedSurf/meshedSurfRef.H       |  5 +-
 src/surfMesh/writers/common/surfaceWriter.C   | 34 +++++++-
 src/surfMesh/writers/common/surfaceWriter.H   |  7 +-
 .../writers/debug/debugSurfaceWriter.C        | 27 +++---
 .../writers/debug/debugSurfaceWriter.H        | 12 +--
 7 files changed, 124 insertions(+), 85 deletions(-)

diff --git a/src/surfMesh/mergedSurf/mergedSurf.C b/src/surfMesh/mergedSurf/mergedSurf.C
index 456281f02a7..ed68acd8e2d 100644
--- a/src/surfMesh/mergedSurf/mergedSurf.C
+++ b/src/surfMesh/mergedSurf/mergedSurf.C
@@ -27,7 +27,6 @@ License
 
 #include "mergedSurf.H"
 #include "PatchTools.H"
-#include "globalIndex.H"
 
 // * * * * * * * * * * * * * * * * Constructors  * * * * * * * * * * * * * * //
 
@@ -36,8 +35,6 @@ Foam::mergedSurf::mergedSurf
     const meshedSurf& unmergedSurface,
     const scalar mergeDim
 )
-:
-    mergedSurf()
 {
     merge(unmergedSurface, mergeDim);
 }
@@ -49,8 +46,6 @@ Foam::mergedSurf::mergedSurf
     const faceList& unmergedFaces,
     const scalar mergeDim
 )
-:
-    mergedSurf()
 {
     merge(unmergedPoints, unmergedFaces, mergeDim);
 }
@@ -64,8 +59,6 @@ Foam::mergedSurf::mergedSurf
     const labelList& origFaceIds,
     const scalar mergeDim
 )
-:
-    mergedSurf()
 {
     merge
     (
@@ -80,12 +73,6 @@ Foam::mergedSurf::mergedSurf
 
 // * * * * * * * * * * * * * * * Member Functions  * * * * * * * * * * * * * //
 
-bool Foam::mergedSurf::use()
-{
-    return Pstream::parRun();
-}
-
-
 void Foam::mergedSurf::clear()
 {
     points_.clear();
@@ -94,6 +81,9 @@ void Foam::mergedSurf::clear()
 
     zoneIds_.clear();
     faceIds_.clear();
+
+    pointGlobalIndex_.clear();
+    faceGlobalIndex_.clear();
 }
 
 
@@ -127,8 +117,8 @@ bool Foam::mergedSurf::merge
         (
             unmergedPoints,
             unmergedFaces,
-            labelList(),
-            labelList(),
+            labelList::null(),
+            labelList::null(),
             mergeDim
         );
 }
@@ -143,9 +133,9 @@ bool Foam::mergedSurf::merge
     const scalar mergeDim
 )
 {
-    if (!use())
+    if (!UPstream::parRun())
     {
-        clear();   // Extra safety?
+        clear();  // Safety
         return false;
     }
 
@@ -155,14 +145,25 @@ bool Foam::mergedSurf::merge
         primitivePatch(SubList<face>(unmergedFaces), unmergedPoints),
         points_,
         faces_,
+        pointGlobalIndex_,
+        faceGlobalIndex_,
         pointsMap_
     );
 
 
-    // Now handle per-face information
+    // The zone/ids information is either *exactly* the same size as
+    // the number of faces, or zero-sized everywhere.
+    // However, use gatherOp anyhow, which has redundant overhead,
+    // but safer if there are any size mis-matches
 
-    globalIndex::gatherOp(origZoneIds, zoneIds_);
-    globalIndex::gatherOp(origFaceIds, faceIds_);
+    if (notNull(origZoneIds))
+    {
+        globalIndex::gatherOp(origZoneIds, zoneIds_);
+    }
+    if (notNull(origFaceIds))
+    {
+        globalIndex::gatherOp(origFaceIds, faceIds_);
+    }
 
     return true;
 }
diff --git a/src/surfMesh/mergedSurf/mergedSurf.H b/src/surfMesh/mergedSurf/mergedSurf.H
index 1b03e8e8743..1fdec662561 100644
--- a/src/surfMesh/mergedSurf/mergedSurf.H
+++ b/src/surfMesh/mergedSurf/mergedSurf.H
@@ -5,7 +5,7 @@
     \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-    Copyright (C) 2016-2020 OpenCFD Ltd.
+    Copyright (C) 2016-2022 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -36,10 +36,12 @@ SourceFiles
 
 \*---------------------------------------------------------------------------*/
 
-#ifndef mergedSurf_H
-#define mergedSurf_H
+#ifndef Foam_mergedSurf_H
+#define Foam_mergedSurf_H
 
 #include "meshedSurf.H"
+#include "globalIndex.H"
+#include "stdFoam.H"
 
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
 
@@ -54,20 +56,28 @@ class mergedSurf
 :
     public meshedSurf
 {
-    pointField points_;
-    faceList   faces_;
-    labelList  pointsMap_;
+    // Private Data
 
-    labelList  zoneIds_;
-    labelList  faceIds_;
+        pointField points_;
+        faceList   faces_;
+        labelList  pointsMap_;
+
+        labelList  zoneIds_;
+        labelList  faceIds_;
+
+        //- Gather/merge information (points)
+        globalIndex pointGlobalIndex_;
+
+        //- Gather/merge information (faces)
+        globalIndex faceGlobalIndex_;
 
 
 public:
 
-    // Constructors
+    // Generated Methods
 
         //- Default construct
-        mergedSurf() = default;
+        mergedSurf() noexcept = default;
 
         //- Copy construct
         mergedSurf(const mergedSurf&) = default;
@@ -75,14 +85,23 @@ public:
         //- Move construct
         mergedSurf(mergedSurf&&) = default;
 
-        //- Construct and merge
+        //- Copy assignment
+        mergedSurf& operator=(const mergedSurf&) = default;
+
+        //- Move assignment
+        mergedSurf& operator=(mergedSurf&&) = default;
+
+
+    // Constructors
+
+        //- Construct from surface, and merge
         mergedSurf
         (
             const meshedSurf& unmergedSurface,
             const scalar mergeDim
         );
 
-        //- Construct and merge
+        //- Construct from points/faces, and merge
         mergedSurf
         (
             const pointField& unmergedPoints,
@@ -90,7 +109,7 @@ public:
             const scalar mergeDim
         );
 
-        //- Construct and merge
+        //- Construct from points/faces/zones/face-ids, and merge
         mergedSurf
         (
             const pointField& unmergedPoints,
@@ -105,47 +124,56 @@ public:
     virtual ~mergedSurf() = default;
 
 
-    // Access Member Functions
-
-        //- Can use (parallel only)
-        static bool use();
+    // Access
 
         //- Number of faces
-        label size() const
+        label size() const noexcept
         {
             return faces_.size();
         }
 
         //- Const access to (global) points used for the surface
-        virtual const pointField& points() const
+        virtual const pointField& points() const noexcept
         {
             return points_;
         }
 
         //- Const access to the surface faces
-        virtual const faceList& faces() const
+        virtual const faceList& faces() const noexcept
         {
             return faces_;
         }
 
         //- Per-face zone/region information
-        virtual const labelList& zoneIds() const
+        virtual const labelList& zoneIds() const noexcept
         {
             return zoneIds_;
         }
 
         //- Per-face identifier (eg, element Id)
-        virtual const labelList& faceIds() const
+        virtual const labelList& faceIds() const noexcept
         {
             return faceIds_;
         }
 
         //- Map for reordered points (old-to-new)
-        const labelList& pointsMap() const
+        const labelList& pointsMap() const noexcept
         {
             return pointsMap_;
         }
 
+        //- Const access to globalIndex used for points gathering
+        const globalIndex& pointGlobalIndex() const noexcept
+        {
+            return pointGlobalIndex_;
+        }
+
+        //- Const access to globalIndex used for faces gathering
+        const globalIndex& faceGlobalIndex() const noexcept
+        {
+            return faceGlobalIndex_;
+        }
+
 
     // Edit
 
@@ -176,15 +204,6 @@ public:
             const labelList& origFaceIds,
             const scalar mergeDim
         );
-
-
-    // Member Operators
-
-        //- Copy assignment
-        mergedSurf& operator=(const mergedSurf&) = default;
-
-        //- Move assignment
-        mergedSurf& operator=(mergedSurf&&) = default;
 };
 
 
diff --git a/src/surfMesh/meshedSurf/meshedSurfRef.H b/src/surfMesh/meshedSurf/meshedSurfRef.H
index a9c8a007aea..157ac14c233 100644
--- a/src/surfMesh/meshedSurf/meshedSurfRef.H
+++ b/src/surfMesh/meshedSurf/meshedSurfRef.H
@@ -114,11 +114,14 @@ public:
     // Member Functions
 
         //- Contains a valid reference?
-        bool valid() const
+        bool good() const
         {
             return (surf_ || notNull(points_.get()));
         }
 
+        //- Contains a valid reference?
+        bool valid() const { return good(); }
+
         //- The original points used for the surface
         const pointField& points0() const
         {
diff --git a/src/surfMesh/writers/common/surfaceWriter.C b/src/surfMesh/writers/common/surfaceWriter.C
index e8868dcd615..80c03939db9 100644
--- a/src/surfMesh/writers/common/surfaceWriter.C
+++ b/src/surfMesh/writers/common/surfaceWriter.C
@@ -30,7 +30,6 @@ License
 #include "MeshedSurfaceProxy.H"
 
 #include "Time.H"
-#include "globalIndex.H"
 #include "coordinateRotation.H"
 #include "transformField.H"
 #include "addToRunTimeSelectionTable.H"
@@ -152,6 +151,7 @@ Foam::surfaceWriter::surfaceWriter()
     useTimeDir_(false),
     isPointData_(false),
     verbose_(false),
+    commType_(UPstream::commsTypes::scheduled),
     nFields_(0),
     currTime_(),
     outputPath_(),
@@ -168,6 +168,8 @@ Foam::surfaceWriter::surfaceWriter(const dictionary& options)
 {
     options.readIfPresent("verbose", verbose_);
 
+    UPstream::commsTypeNames.readIfPresent("commsType", options, commType_);
+
     geometryScale_ = 1;
     geometryCentre_ = Zero;
     geometryTransform_.clear();
@@ -188,6 +190,13 @@ Foam::surfaceWriter::surfaceWriter(const dictionary& options)
 
     fieldLevel_ = options.subOrEmptyDict("fieldLevel");
     fieldScale_ = options.subOrEmptyDict("fieldScale");
+
+    if (verbose_)
+    {
+        Info<< "Create surfaceWriter ("
+            << (this->isPointData() ? "point" : "face") << " data):"
+            << " commsType=" << UPstream::commsTypeNames[commType_] << endl;
+    }
 }
 
 
@@ -393,7 +402,7 @@ bool Foam::surfaceWriter::expire()
 
 bool Foam::surfaceWriter::hasSurface() const
 {
-    return surf_.valid();
+    return surf_.good();
 }
 
 
@@ -430,6 +439,7 @@ bool Foam::surfaceWriter::merge() const
 
     if (!upToDate_)
     {
+        // Similar to expire
         adjustedSurf_.clear();
 
         if (parallel_ && Pstream::parRun())
@@ -473,7 +483,7 @@ const Foam::meshedSurfRef& Foam::surfaceWriter::adjustSurface() const
         adjustedSurf_.clear();
     }
 
-    if (!adjustedSurf_.valid())
+    if (!adjustedSurf_.good())
     {
         adjustedSurf_.reset(surface());
 
@@ -538,7 +548,23 @@ Foam::tmp<Foam::Field<Type>> Foam::surfaceWriter::mergeFieldTemplate
         auto tfield = tmp<Field<Type>>::New();
         auto& allFld = tfield.ref();
 
-        globalIndex::gatherOp(fld, allFld);
+        // Update any expired global index (as required)
+
+        const globalIndex& globIndex =
+        (
+            this->isPointData()
+          ? mergedSurf_.pointGlobalIndex()
+          : mergedSurf_.faceGlobalIndex()
+        );
+
+        globIndex.gather
+        (
+            fld,
+            allFld,
+            UPstream::msgType(),
+            commType_,
+            UPstream::worldComm
+        );
 
         // Renumber (point data) to correspond to merged points
         if
diff --git a/src/surfMesh/writers/common/surfaceWriter.H b/src/surfMesh/writers/common/surfaceWriter.H
index c4535a01548..e4d4e345788 100644
--- a/src/surfMesh/writers/common/surfaceWriter.H
+++ b/src/surfMesh/writers/common/surfaceWriter.H
@@ -71,10 +71,11 @@ Description
     }
     \endverbatim
 
-  Format options:
+    Format options:
     \table
         Property    | Description                           | Reqd | Default
         verbose     | Additional output verbosity           | no  | no
+        commsType   | Communication type                    | no  | scheduled
         scale       | Output geometry scaling               | no  | 1
         transform   | Output coordinate transform           | no  |
         fieldLevel  | Subtract field level before scaling   | no  | empty dict
@@ -108,6 +109,7 @@ SourceFiles
 #include "instant.H"
 #include "mergedSurf.H"
 #include "meshedSurfRef.H"
+#include "UPstream.H"
 #include "InfoProxy.H"
 #include "runTimeSelectionTables.H"
 
@@ -180,6 +182,9 @@ protected:
         //- Additional output verbosity
         bool verbose_;
 
+        //- Communication type (for field merging)
+        UPstream::commsTypes commType_;
+
         //- The number of fields
         label nFields_;
 
diff --git a/src/surfMesh/writers/debug/debugSurfaceWriter.C b/src/surfMesh/writers/debug/debugSurfaceWriter.C
index 796136b5559..7d1ee11c48b 100644
--- a/src/surfMesh/writers/debug/debugSurfaceWriter.C
+++ b/src/surfMesh/writers/debug/debugSurfaceWriter.C
@@ -82,17 +82,23 @@ Foam::surfaceWriters::debugWriter::mergeField
                 fld,
                 allFld,
                 UPstream::worldComm,
-                commType_,
-                (UPstream::msgType() + msgTag_)
+                commType_
             );
         }
         else
         {
-            globalIndex::gatherOp
+            const globalIndex& globIndex =
+            (
+                this->isPointData()
+              ? mergedSurf_.pointGlobalIndex()
+              : mergedSurf_.faceGlobalIndex()
+            );
+
+            globIndex.gather
             (
                 fld,
                 allFld,
-                (UPstream::msgType() + msgTag_),
+                UPstream::msgType(),
                 commType_,
                 UPstream::worldComm
             );
@@ -125,11 +131,9 @@ Foam::surfaceWriters::debugWriter::mergeField
 Foam::surfaceWriters::debugWriter::debugWriter()
 :
     surfaceWriter(),
-    commType_(UPstream::commsTypes::nonBlocking),
     mpiGatherv_(false),
     enableWrite_(false),
     header_(true),
-    msgTag_(0),
     streamOpt_(IOstreamOption::BINARY)
 {}
 
@@ -140,25 +144,14 @@ Foam::surfaceWriters::debugWriter::debugWriter
 )
 :
     surfaceWriter(options),
-    commType_
-    (
-        UPstream::commsTypeNames.getOrDefault
-        (
-            "commsType",
-            options,
-            UPstream::commsTypes::nonBlocking
-        )
-    ),
     mpiGatherv_(options.getOrDefault("gatherv", false)),
     enableWrite_(options.getOrDefault("write", false)),
     header_(true),
-    msgTag_(options.getOrDefault<int>("msgTag", 0)),
     streamOpt_(IOstreamOption::BINARY)
 {
     Info<< "Using debug surface writer ("
         << (this->isPointData() ? "point" : "face") << " data):"
         << " commsType=" << UPstream::commsTypeNames[commType_]
-        << " msgTag=" << msgTag_
         << " gatherv=" << Switch::name(mpiGatherv_)
         << " write=" << Switch::name(enableWrite_) << endl;
 }
diff --git a/src/surfMesh/writers/debug/debugSurfaceWriter.H b/src/surfMesh/writers/debug/debugSurfaceWriter.H
index cbd707b0e01..cb64f2de99d 100644
--- a/src/surfMesh/writers/debug/debugSurfaceWriter.H
+++ b/src/surfMesh/writers/debug/debugSurfaceWriter.H
@@ -43,10 +43,9 @@ Description
     Format options:
     \table
         Property    | Description                           | Required | Default
-        write       | Write boundaryData (binary with header)  | no  | false
-        commsType   | blocking/nonBlocking/scheduled        | no  | nonBlocking
-        msgTag      | increment for the message tag         | no  | 0
+        commsType   | blocking/nonBlocking/scheduled        | no  | scheduled
         gatherv     | Use MPI gatherv when merging          | no  | false
+        write       | Write boundaryData (binary + header)  | no  | false
     \endtable
 
 SourceFiles
@@ -58,7 +57,6 @@ SourceFiles
 #define Foam_debugSurfaceWriter_H
 
 #include "surfaceWriter.H"
-#include "UPstream.H"
 
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
 
@@ -81,9 +79,6 @@ class debugWriter
 {
     // Private Data
 
-        //- Communication type for merging
-        UPstream::commsTypes commType_;
-
         //- Use MPI gatherv in globalIndex gathering
         bool mpiGatherv_;
 
@@ -93,9 +88,6 @@ class debugWriter
         //- Output files with FoamFile header
         bool header_;
 
-        //- Increment for the message tag
-        int msgTag_;
-
         //- Output stream option
         IOstreamOption streamOpt_;
 
-- 
GitLab