STYLE: minor cleanup in decompositionMethod

- reduce cell looping. Avoid initial looping over blockFace.

- make early return (no processor sets, connections, or blocked faces)
  more apparent.

src/parallel/decompose/decompositionMethods/decompositionMethod/decompositionMethod.C

@@ -31,6 +31,7 @@ License
 #include "regionSplit.H"
 #include "localPointRegion.H"
 #include "minData.H"
+#include "BitOps.H"
 #include "FaceCellWave.H"
 
 #include "preserveBafflesConstraint.H"
@@ -48,7 +49,8 @@ namespace Foam
 
     // Fallback name when searching for optional coefficients directories
     static const word defaultName("coeffs");
-}
+
+} // End namespace Foam
 
 
 // * * * * * * * * * * * * * Static Member Functions * * * * * * * * * * * * //
@@ -654,15 +656,15 @@ void Foam::decompositionMethod::calcCellCells
     // Check for duplicates connections between cells
     // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     // Done as postprocessing step since we now have cellCells.
-    label newIndex = 0;
-    labelHashSet nbrCells;
-
 
     if (cellCells.size() == 0)
     {
         return;
     }
 
+    label newIndex = 0;
+    labelHashSet nbrCells;
+
     label startIndex = cellCells.offsets()[0];
 
     forAll(cellCells, celli)
@@ -783,8 +785,8 @@ void Foam::decompositionMethod::calcCellCells
             forAll(pp, i)
             {
                 const label own = agglom[faceOwner[facei]];
-
                 const label globalNei = globalNeighbour[bFacei];
+
                 if
                 (
                    !globalAgglom.isLocal(globalNei)
@@ -839,7 +841,6 @@ void Foam::decompositionMethod::calcCellCells
             forAll(pp, i)
             {
                 const label own = agglom[faceOwner[facei]];
-
                 const label globalNei = globalNeighbour[bFacei];
 
                 if
@@ -848,7 +849,7 @@ void Foam::decompositionMethod::calcCellCells
                  || globalAgglom.toLocal(globalNei) != own
                 )
                 {
-                    label ownIndex = offsets[own] + nFacesPerCell[own]++;
+                    const label ownIndex = offsets[own] + nFacesPerCell[own]++;
                     m[ownIndex] = globalNei;
                     w[ownIndex] = mag(mesh.faceAreas()[facei]);
                 }
@@ -863,15 +864,15 @@ void Foam::decompositionMethod::calcCellCells
     // Check for duplicates connections between cells
     // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     // Done as postprocessing step since we now have cellCells.
-    label newIndex = 0;
-    labelHashSet nbrCells;
-
 
     if (cellCells.size() == 0)
     {
         return;
     }
 
+    label newIndex = 0;
+    labelHashSet nbrCells;
+
     label startIndex = cellCells.offsets()[0];
 
     forAll(cellCells, celli)
@@ -923,9 +924,9 @@ Foam::labelList Foam::decompositionMethod::decompose
 ) const
 {
     // Any weights specified?
-    label nWeights = returnReduce(cellWeights.size(), sumOp<label>());
+    const bool hasWeights = returnReduce(!cellWeights.empty(), orOp<bool>());
 
-    if (nWeights > 0 && cellWeights.size() != mesh.nCells())
+    if (hasWeights && cellWeights.size() != mesh.nCells())
     {
         FatalErrorInFunction
             << "Number of weights " << cellWeights.size()
@@ -933,128 +934,126 @@ Foam::labelList Foam::decompositionMethod::decompose
             << exit(FatalError);
     }
 
+    // Any faces not blocked?
+    const bool hasUnblocked =
+        returnReduce
+        (
+            (!blockedFace.empty() && !BitOps::all(blockedFace)),
+            orOp<bool>()
+        );
 
-    // Any processor sets?
-    label nProcSets = 0;
-    forAll(specifiedProcessorFaces, setI)
-    {
-        nProcSets += specifiedProcessorFaces[setI].size();
-    }
-    reduce(nProcSets, sumOp<label>());
 
     // Any non-mesh connections?
-    label nConnections = returnReduce
+    const label nConnections = returnReduce
     (
         explicitConnections.size(),
         sumOp<label>()
     );
 
-    // Any faces not blocked?
-    label nUnblocked = 0;
-    forAll(blockedFace, facei)
-    {
-        if (!blockedFace[facei])
-        {
-            nUnblocked++;
-        }
-    }
-    reduce(nUnblocked, sumOp<label>());
 
+    // Any processor sets?
+    label nProcSets = 0;
+    for (const labelList& procset : specifiedProcessorFaces)
+    {
+        nProcSets += procset.size();
+    }
+    reduce(nProcSets, sumOp<label>());
 
 
     // Either do decomposition on cell centres or on agglomeration
 
-    labelList finalDecomp;
+    if (!hasUnblocked && !nConnections && !nProcSets)
-
-
-    if (nProcSets+nConnections+nUnblocked == 0)
     {
         // No constraints, possibly weights
 
-        if (nWeights > 0)
+        return
-        {
+        (
-            finalDecomp = decompose
+            hasWeights
-            (
+          ? decompose(mesh, mesh.cellCentres(), cellWeights)
-                mesh,
+          : decompose(mesh, mesh.cellCentres())
-                mesh.cellCentres(),
+        );
-                cellWeights
-            );
-        }
-        else
-        {
-            finalDecomp = decompose(mesh, mesh.cellCentres());
-        }
     }
-    else
+
+
+    // The harder work.
+    // When we have processor sets, connections, or blocked faces.
+
+
+    // Determine local regions, separated by blockedFaces
+    regionSplit localRegion(mesh, blockedFace, explicitConnections, false);
+
+    if (debug)
     {
-        if (debug)
+        // Only need to count unblocked faces for debugging
-        {
+        const label nUnblocked =
-            Info<< "Constrained decomposition:" << endl
+        (
-                << "    faces with same owner and neighbour processor : "
+            hasUnblocked
-                << nUnblocked << endl
+          ? returnReduce
-                << "    baffle faces with same owner processor        : "
+            (
-                << nConnections << endl
+                label(BitOps::count(blockedFace, false)),
-                << "    faces all on same processor                   : "
+                sumOp<label>()
-                << nProcSets << endl << endl;
+            )
-        }
+          : 0
+        );
 
-        // Determine local regions, separated by blockedFaces
+        Info<< "Constrained decomposition:" << nl
-        regionSplit localRegion(mesh, blockedFace, explicitConnections, false);
+            << "    faces with same owner and neighbour processor : "
+            << nUnblocked << nl
+            << "    baffle faces with same owner processor        : "
+            << nConnections << nl
+            << "    faces all on same processor                   : "
+            << nProcSets << nl
+            << "    split into " << localRegion.nLocalRegions()
+            << " regions."
+            << endl;
+    }
 
 
-        if (debug)
+    // Gather region weights and determine region cell centres
-        {
+    // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-            Info<< "Constrained decomposition:" << endl
-                << "    split into " << localRegion.nLocalRegions()
-                << " regions."
-                << endl;
-        }
 
-        // Determine region cell centres
+    // For the region centre, just take the first cell in the region.
-        // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+    // If we average the region centre instead, cyclics could cause
+    // the average domain centre to be outside of domain.
 
-        // This just takes the first cell in the region. Otherwise the problem
+    scalarField regionWeights(localRegion.nLocalRegions(), 0.0);
-        // is with cyclics - if we'd average the region centre might be
-        // somewhere in the middle of the domain which might not be anywhere
-        // near any of the cells.
 
-        pointField regionCentres(localRegion.nLocalRegions(), point::max);
+    pointField regionCentres(localRegion.nLocalRegions(), point::max);
 
+    if (hasWeights)
+    {
         forAll(localRegion, celli)
         {
             const label regioni = localRegion[celli];
 
+            regionWeights[regioni] += cellWeights[celli];
+
             if (regionCentres[regioni] == point::max)
             {
                 regionCentres[regioni] = mesh.cellCentres()[celli];
             }
         }
-
+    }
-        // Do decomposition on agglomeration
+    else
-        // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+    {
-
+        forAll(localRegion, celli)
-        scalarField regionWeights(localRegion.nLocalRegions(), 0);
-
-        if (nWeights > 0)
         {
-            forAll(localRegion, celli)
+            const label regioni = localRegion[celli];
-            {
-                const label regioni = localRegion[celli];
 
-                regionWeights[regioni] += cellWeights[celli];
+            regionWeights[regioni] += 1.0;
+
+            if (regionCentres[regioni] == point::max)
+            {
+                regionCentres[regioni] = mesh.cellCentres()[celli];
             }
         }
-        else
+    }
-        {
-            forAll(localRegion, celli)
-            {
-                const label regioni = localRegion[celli];
 
-                regionWeights[regioni] += 1.0;
+    // Do decomposition on agglomeration
-            }
+    // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-        }
 
-        finalDecomp = decompose
+    labelList finalDecomp =
+        decompose
         (
             mesh,
             localRegion,
@@ -1063,189 +1062,188 @@ Foam::labelList Foam::decompositionMethod::decompose
         );
 
 
+    // Apply explicitConnections since decompose did not know about them
+    for (const labelPair& baffle : explicitConnections)
+    {
+        const label f0 = baffle.first();
+        const label f1 = baffle.second();
 
-        // Implement the explicitConnections since above decompose
+        if (!blockedFace[f0] && !blockedFace[f1])
-        // does not know about them
-        forAll(explicitConnections, connectioni)
         {
-            const labelPair& baffle = explicitConnections[connectioni];
+            // Note: what if internal faces and owner and neighbour on
-            const label f0 = baffle.first();
+            // different processor?
-            const label f1 = baffle.second();
+            // So for now just push owner side proc
 
-            if (!blockedFace[f0] && !blockedFace[f1])
+            const label proci = finalDecomp[mesh.faceOwner()[f0]];
+
+            finalDecomp[mesh.faceOwner()[f1]] = proci;
+            if (mesh.isInternalFace(f1))
             {
-                // Note: what if internal faces and owner and neighbour on
+                finalDecomp[mesh.faceNeighbour()[f1]] = proci;
-                // different processor? So for now just push owner side
-                // proc
-
-                const label proci = finalDecomp[mesh.faceOwner()[f0]];
-
-                finalDecomp[mesh.faceOwner()[f1]] = proci;
-                if (mesh.isInternalFace(f1))
-                {
-                    finalDecomp[mesh.faceNeighbour()[f1]] = proci;
-                }
             }
-            else if (blockedFace[f0] != blockedFace[f1])
+        }
+        else if (blockedFace[f0] != blockedFace[f1])
+        {
+            FatalErrorInFunction
+                << "On explicit connection between faces " << f0
+                << " and " << f1
+                << " the two blockedFace status are not equal : "
+                << blockedFace[f0] << " and " << blockedFace[f1]
+                << exit(FatalError);
+        }
+    }
+
+
+    // blockedFaces corresponding to processor faces need to be handled
+    // separately since not handled by local regionSplit. We need to
+    // walk now across coupled faces and make sure to move a whole
+    // global region across
+
+    // This additionally consolidates/compacts the regions numbers globally,
+    // since that was skipped in the previous regionSplit.
+    if (Pstream::parRun())
+    {
+        // Re-do regionSplit
+
+        // Field on cells and faces.
+        List<minData> cellData(mesh.nCells());
+        List<minData> faceData(mesh.nFaces());
+
+        // Take over blockedFaces by seeding a negative number
+        // (so is always less than the decomposition)
+        label nUnblocked = 0;
+        forAll(blockedFace, facei)
+        {
+            if (blockedFace[facei])
             {
-                FatalErrorInFunction
+                faceData[facei] = minData(-123);
-                    << "On explicit connection between faces " << f0
+            }
-                    << " and " << f1
+            else
-                    << " the two blockedFace status are not equal : "
+            {
-                    << blockedFace[f0] << " and " << blockedFace[f1]
+                ++nUnblocked;
-                    << exit(FatalError);
+            }
+        }
+
+        // Seed unblocked faces with destination processor
+        labelList seedFaces(nUnblocked);
+        List<minData> seedData(nUnblocked);
+        nUnblocked = 0;
+
+        forAll(blockedFace, facei)
+        {
+            if (!blockedFace[facei])
+            {
+                const label own = mesh.faceOwner()[facei];
+                seedFaces[nUnblocked] = facei;
+                seedData[nUnblocked] = minData(finalDecomp[own]);
+                nUnblocked++;
             }
         }
 
 
-        // blockedFaces corresponding to processor faces need to be handled
+        // Propagate information inwards
-        // separately since not handled by local regionSplit. We need to
+        FaceCellWave<minData> deltaCalc
-        // walk now across coupled faces and make sure to move a whole
+        (
-        // global region across
+            mesh,
-        if (Pstream::parRun())
+            seedFaces,
+            seedData,
+            faceData,
+            cellData,
+            mesh.globalData().nTotalCells()+1
+        );
+
+        // And extract
+        forAll(finalDecomp, celli)
         {
-            // Re-do regionSplit
+            if (cellData[celli].valid(deltaCalc.data()))
-
-            // Field on cells and faces.
-            List<minData> cellData(mesh.nCells());
-            List<minData> faceData(mesh.nFaces());
-
-            // Take over blockedFaces by seeding a negative number
-            // (so is always less than the decomposition)
-            label nUnblocked = 0;
-            forAll(blockedFace, facei)
             {
-                if (blockedFace[facei])
+                finalDecomp[celli] = cellData[celli].data();
-                {
-                    faceData[facei] = minData(-123);
-                }
-                else
-                {
-                    nUnblocked++;
-                }
             }
+        }
+    }
 
-            // Seed unblocked faces with destination processor
-            labelList seedFaces(nUnblocked);
-            List<minData> seedData(nUnblocked);
-            nUnblocked = 0;
 
-            forAll(blockedFace, facei)
+    // For specifiedProcessorFaces rework the cellToProc to enforce
+    // all on one processor since we can't guarantee that the input
+    // to regionSplit was a single region.
+    // E.g. faceSet 'a' with the cells split into two regions
+    // by a notch formed by two walls
+    //
+    //          \   /
+    //           \ /
+    //    ---a----+-----a-----
+    //
+    //
+    // Note that reworking the cellToProc might make the decomposition
+    // unbalanced.
+    forAll(specifiedProcessorFaces, seti)
+    {
+        const labelList& set = specifiedProcessorFaces[seti];
+
+        label proci = specifiedProcessor[seti];
+        if (proci == -1)
+        {
+            // If no processor specified - use the one from the 0th element
+            if (set.size())
             {
-                if (!blockedFace[facei])
+                proci = finalDecomp[mesh.faceOwner()[set[0]]];
-                {
-                    const label own = mesh.faceOwner()[facei];
-                    seedFaces[nUnblocked] = facei;
-                    seedData[nUnblocked] = minData(finalDecomp[own]);
-                    nUnblocked++;
-                }
             }
-
+            else
-
-            // Propagate information inwards
-            FaceCellWave<minData> deltaCalc
-            (
-                mesh,
-                seedFaces,
-                seedData,
-                faceData,
-                cellData,
-                mesh.globalData().nTotalCells()+1
-            );
-
-            // And extract
-            forAll(finalDecomp, celli)
             {
-                if (cellData[celli].valid(deltaCalc.data()))
+                // Zero-sized processor (e.g. from redistributePar)
-                {
+                proci = 0;
-                    finalDecomp[celli] = cellData[celli].data();
-                }
             }
         }
 
-
+        for (const label facei : set)
-        // For specifiedProcessorFaces rework the cellToProc to enforce
-        // all on one processor since we can't guarantee that the input
-        // to regionSplit was a single region.
-        // E.g. faceSet 'a' with the cells split into two regions
-        // by a notch formed by two walls
-        //
-        //          \   /
-        //           \ /
-        //    ---a----+-----a-----
-        //
-        //
-        // Note that reworking the cellToProc might make the decomposition
-        // unbalanced.
-        forAll(specifiedProcessorFaces, setI)
         {
-            const labelList& set = specifiedProcessorFaces[setI];
+            const face& f = mesh.faces()[facei];
-
+            for (const label pointi : f)
-            label proci = specifiedProcessor[setI];
-            if (proci == -1)
             {
-                // If no processor specified use the one from the
+                const labelList& pFaces = mesh.pointFaces()[pointi];
-                // 0th element
+                for (const label pFacei : pFaces)
-                if (set.size())
                 {
-                    proci = finalDecomp[mesh.faceOwner()[set[0]]];
+                    finalDecomp[mesh.faceOwner()[pFacei]] = proci;
-                }
+                    if (mesh.isInternalFace(pFacei))
-                else
-                {
-                    // Zero-sized processor (e.g. from redistributePar)
-                    proci = 0;
-                }
-            }
-
-            forAll(set, fI)
-            {
-                const face& f = mesh.faces()[set[fI]];
-                forAll(f, fp)
-                {
-                    const labelList& pFaces = mesh.pointFaces()[f[fp]];
-                    for (const label facei : pFaces)
                     {
-                        finalDecomp[mesh.faceOwner()[facei]] = proci;
+                        finalDecomp[mesh.faceNeighbour()[pFacei]] = proci;
-                        if (mesh.isInternalFace(facei))
-                        {
-                            finalDecomp[mesh.faceNeighbour()[facei]] = proci;
-                        }
                     }
                 }
             }
         }
+    }
 
 
-        if (debug && Pstream::parRun())
+    if (debug && Pstream::parRun())
+    {
+        labelList nbrDecomp;
+        syncTools::swapBoundaryCellList(mesh, finalDecomp, nbrDecomp);
+
+        const polyBoundaryMesh& patches = mesh.boundaryMesh();
+        for (const polyPatch& pp : patches)
         {
-            labelList nbrDecomp;
+            if (pp.coupled())
-            syncTools::swapBoundaryCellList(mesh, finalDecomp, nbrDecomp);
-
-            const polyBoundaryMesh& patches = mesh.boundaryMesh();
-            for (const polyPatch& pp : patches)
             {
-                if (pp.coupled())
+                forAll(pp, i)
                 {
-                    forAll(pp, i)
+                    const label facei = pp.start()+i;
-                    {
+                    const label own = mesh.faceOwner()[facei];
-                        label facei = pp.start()+i;
+                    const label bFacei = facei-mesh.nInternalFaces();
-                        label own = mesh.faceOwner()[facei];
-                        label bFacei = facei-mesh.nInternalFaces();
 
-                        if (!blockedFace[facei])
+                    if (!blockedFace[facei])
+                    {
+                        const label ownProc = finalDecomp[own];
+                        const label nbrProc = nbrDecomp[bFacei];
+
+                        if (ownProc != nbrProc)
                         {
-                            label ownProc = finalDecomp[own];
+                            FatalErrorInFunction
-                            label nbrProc = nbrDecomp[bFacei];
+                                << "patch:" << pp.name()
-                            if (ownProc != nbrProc)
+                                << " face:" << facei
-                            {
+                                << " at:" << mesh.faceCentres()[facei]
-                                FatalErrorInFunction
+                                << " ownProc:" << ownProc
-                                    << "patch:" << pp.name()
+                                << " nbrProc:" << nbrProc
-                                    << " face:" << facei
+                                << exit(FatalError);
-                                    << " at:" << mesh.faceCentres()[facei]
-                                    << " ownProc:" << ownProc
-                                    << " nbrProc:" << nbrProc
-                                    << exit(FatalError);
-                            }
                         }
                     }
                 }