From d5e3f0637697ed8a8ef2f2bf184e9ad8eb3d7d36 Mon Sep 17 00:00:00 2001 From: Mark Olesen <Mark.Olesen@esi-group.com> Date: Thu, 20 Feb 2025 13:54:40 +0100 Subject: [PATCH] COMP: remove 'special purpose' minMaxOp (#3326) - replace with plusOp for reductions. The old use didn't always work well with compiler resolution in all cases. Simplify touse plusOp, and mark the old version as 'delete'. This doesn't not affect very much code. COMP: incorrect include ordering for GeometricFieldFunctions - header was included after the template code STYLE: use UPstream::commWarn(...) setter method --- applications/test/minMax1/Test-minMax1.C | 3 -- .../mesh/manipulation/checkMesh/checkTools.C | 2 +- .../GeometricField/GeometricField.H | 3 +- .../GeometricField/GeometricFieldFunctions.C | 26 ++++++------- .../GeometricField/GeometricFieldFunctions.H | 15 +++---- .../primitives/ranges/MinMax/MinMax.H | 9 +++-- .../primitives/ranges/MinMax/MinMaxI.H | 15 +++++-- .../primitives/ranges/MinMax/MinMaxOps.H | 39 +++++-------------- .../faMesh/faMeshTools/faMeshToolsChecks.C | 8 ++-- .../singleDirectionUniformBin.C | 2 +- .../binModels/uniformBin/uniformBin.C | 8 +--- .../cyclicAMIGAMGInterface.C | 10 ++--- 12 files changed, 60 insertions(+), 80 deletions(-) diff --git a/applications/test/minMax1/Test-minMax1.C b/applications/test/minMax1/Test-minMax1.C index 86ff65d720a..10a53680b2d 100644 --- a/applications/test/minMax1/Test-minMax1.C +++ b/applications/test/minMax1/Test-minMax1.C @@ -182,10 +182,7 @@ int main(int argc, char *argv[]) minmax1 += values1; Pout<<"range: " << minmax1 << endl; - - Info<< "Reduced: "<< returnReduce(minmax1, plusOp<scalarMinMax>()) << nl; - Info<< "Reduced: "<< returnReduce(minmax1, minMaxOp<scalar>()) << nl; // Info<< "gMinMax: "<< gMinMax(values1v) << nl; diff --git a/applications/utilities/mesh/manipulation/checkMesh/checkTools.C b/applications/utilities/mesh/manipulation/checkMesh/checkTools.C index e13ef311335..8b6b0d3195d 100644 --- a/applications/utilities/mesh/manipulation/checkMesh/checkTools.C +++ b/applications/utilities/mesh/manipulation/checkMesh/checkTools.C @@ -123,7 +123,7 @@ void Foam::printMeshStats(const polyMesh& mesh, const bool allTopology) { // Number of global patches and min-max range of total patches Info<< mesh.boundaryMesh().nNonProcessor() << ' ' - << returnReduce(labelMinMax(nPatches), minMaxOp<label>()) << nl; + << returnReduce(labelMinMax(nPatches), sumOp<labelMinMax>{}) << nl; } else { diff --git a/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricField.H b/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricField.H index 8cb500f580c..eca2f4e4e93 100644 --- a/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricField.H +++ b/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricField.H @@ -1076,13 +1076,12 @@ public: // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * // #include "GeometricFieldI.H" +#include "GeometricFieldFunctions.H" #ifdef NoRepository #include "GeometricField.C" #endif -#include "GeometricFieldFunctions.H" - // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * // #endif diff --git a/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricFieldFunctions.C b/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricFieldFunctions.C index 40ab6e76461..8048be9d6d7 100644 --- a/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricFieldFunctions.C +++ b/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricFieldFunctions.C @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2018-2023 OpenCFD Ltd. + Copyright (C) 2018-2025 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -461,13 +461,14 @@ dimensioned<ReturnType> Func \ UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(Type, max, maxOp) UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(Type, min, minOp) -UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(MinMax<Type>, minMax, minMaxOp) -UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(scalarMinMax, minMaxMag, minMaxMagOp) +UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(MinMax<Type>, minMax, plusOp) +UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(scalarMinMax, minMaxMag, plusOp) #undef UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY -#define UNARY_REDUCTION_FUNCTION(ReturnType, Func, gFunc) \ +// Forward to DimensionedField directly (same name) +#define UNARY_REDUCTION_FUNCTION(ReturnType, Func) \ \ template<class Type, template<class> class PatchField, class GeoMesh> \ dimensioned<ReturnType> Func \ @@ -475,12 +476,7 @@ dimensioned<ReturnType> Func \ const GeometricField<Type, PatchField, GeoMesh>& f1 \ ) \ { \ - return dimensioned<ReturnType> \ - ( \ - #Func "(" + f1.name() + ')', \ - f1.dimensions(), \ - gFunc(f1.primitiveField()) \ - ); \ + return Func(f1.internalField()); \ } \ \ template<class Type, template<class> class PatchField, class GeoMesh> \ @@ -489,14 +485,14 @@ dimensioned<ReturnType> Func \ const tmp<GeometricField<Type, PatchField, GeoMesh>>& tf1 \ ) \ { \ - dimensioned<ReturnType> res = Func(tf1()); \ + auto result = Func(tf1()); \ tf1.clear(); \ - return res; \ + return result; \ } -UNARY_REDUCTION_FUNCTION(Type, sum, gSum) -UNARY_REDUCTION_FUNCTION(Type, average, gAverage) -UNARY_REDUCTION_FUNCTION(typename typeOfMag<Type>::type, sumMag, gSumMag) +UNARY_REDUCTION_FUNCTION(Type, sum) +UNARY_REDUCTION_FUNCTION(Type, average) +UNARY_REDUCTION_FUNCTION(typename typeOfMag<Type>::type, sumMag) #undef UNARY_REDUCTION_FUNCTION diff --git a/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricFieldFunctions.H b/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricFieldFunctions.H index 65863a90c02..93339f2688c 100644 --- a/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricFieldFunctions.H +++ b/src/OpenFOAM/fields/GeometricFields/GeometricField/GeometricFieldFunctions.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2018-2023 OpenCFD Ltd. + Copyright (C) 2018-2025 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -236,14 +236,15 @@ dimensioned<ReturnType> Func \ UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(Type, max, maxOp) UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(Type, min, minOp) -UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(MinMax<Type>, minMax, minMaxOp) -UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(scalarMinMax, minMaxMag, minMaxMagOp) +UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(MinMax<Type>, minMax, plusOp) +UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY(scalarMinMax, minMaxMag, plusOp) #undef UNARY_REDUCTION_FUNCTION_WITH_BOUNDARY -#define UNARY_REDUCTION_FUNCTION(ReturnType, Func, gFunc) \ +#define UNARY_REDUCTION_FUNCTION(ReturnType, Func) \ \ +/*! \brief Forwards to Func on internalField */ \ template<class Type, template<class> class PatchField, class GeoMesh> \ dimensioned<ReturnType> Func \ ( \ @@ -256,9 +257,9 @@ dimensioned<ReturnType> Func \ const tmp<GeometricField<Type, PatchField, GeoMesh>>& tf1 \ ); -UNARY_REDUCTION_FUNCTION(Type, sum, gSum) -UNARY_REDUCTION_FUNCTION(Type, average, gAverage) -UNARY_REDUCTION_FUNCTION(typename typeOfMag<Type>::type, sumMag, gSumMag) +UNARY_REDUCTION_FUNCTION(Type, sum) +UNARY_REDUCTION_FUNCTION(Type, average) +UNARY_REDUCTION_FUNCTION(typename typeOfMag<Type>::type, sumMag) #undef UNARY_REDUCTION_FUNCTION diff --git a/src/OpenFOAM/primitives/ranges/MinMax/MinMax.H b/src/OpenFOAM/primitives/ranges/MinMax/MinMax.H index 377cb6d3e6b..4a86bc82484 100644 --- a/src/OpenFOAM/primitives/ranges/MinMax/MinMax.H +++ b/src/OpenFOAM/primitives/ranges/MinMax/MinMax.H @@ -5,7 +5,7 @@ \\ / A nd | www.openfoam.com \\/ M anipulation | ------------------------------------------------------------------------------- - Copyright (C) 2019-2023 OpenCFD Ltd. + Copyright (C) 2019-2025 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -241,6 +241,9 @@ public: //- Include the value into the range inline MinMax<T>& add(const T& val); + //- Include two values into the range + inline MinMax<T>& add(const T& val1, const T& val2); + //- Include the values into the range inline MinMax<T>& add(const UList<T>& vals); @@ -264,10 +267,10 @@ public: inline MinMax<T>& operator+=(const UList<T>& vals); //- Multiply range by scalar factor - inline MinMax<T>& operator*=(const scalar& s); + inline MinMax<T>& operator*=(scalar s); //- Divide range by scalar factor - inline MinMax<T>& operator/=(const scalar& s); + inline MinMax<T>& operator/=(scalar s); // Housekeeping diff --git a/src/OpenFOAM/primitives/ranges/MinMax/MinMaxI.H b/src/OpenFOAM/primitives/ranges/MinMax/MinMaxI.H index 444c6853204..724de3f1b97 100644 --- a/src/OpenFOAM/primitives/ranges/MinMax/MinMaxI.H +++ b/src/OpenFOAM/primitives/ranges/MinMax/MinMaxI.H @@ -5,7 +5,7 @@ \\ / A nd | www.openfoam.com \\/ M anipulation | ------------------------------------------------------------------------------- - Copyright (C) 2019-2023 OpenCFD Ltd. + Copyright (C) 2019-2025 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -271,6 +271,15 @@ inline Foam::MinMax<T>& Foam::MinMax<T>::add(const T& val) } +template<class T> +inline Foam::MinMax<T>& Foam::MinMax<T>::add(const T& val1, const T& val2) +{ + add(val1); + add(val2); + return *this; +} + + template<class T> inline Foam::MinMax<T>& Foam::MinMax<T>::add(const UList<T>& vals) { @@ -354,7 +363,7 @@ inline Foam::MinMax<T>& Foam::MinMax<T>::operator+=(const UList<T>& vals) template<class T> -inline Foam::MinMax<T>& Foam::MinMax<T>::operator*=(const scalar& s) +inline Foam::MinMax<T>& Foam::MinMax<T>::operator*=(scalar s) { min() *= s; max() *= s; @@ -363,7 +372,7 @@ inline Foam::MinMax<T>& Foam::MinMax<T>::operator*=(const scalar& s) template<class T> -inline Foam::MinMax<T>& Foam::MinMax<T>::operator/=(const scalar& s) +inline Foam::MinMax<T>& Foam::MinMax<T>::operator/=(scalar s) { min() /= s; max() /= s; diff --git a/src/OpenFOAM/primitives/ranges/MinMax/MinMaxOps.H b/src/OpenFOAM/primitives/ranges/MinMax/MinMaxOps.H index 5bad8452ec1..b09841aae67 100644 --- a/src/OpenFOAM/primitives/ranges/MinMax/MinMaxOps.H +++ b/src/OpenFOAM/primitives/ranges/MinMax/MinMaxOps.H @@ -44,7 +44,7 @@ Description namespace Foam { -// Global Functions +// * * * * * * * * * * * * * * * Global Functions * * * * * * * * * * * * * // //- The mag() function for min/max range template<class T> @@ -153,10 +153,8 @@ struct minMaxOp return MinMax<T>(y).add(x); } - MinMax<T> operator()(const MinMax<T>& x, const MinMax<T>& y) const - { - return MinMax<T>(x).add(y); // Same as (x + y) - } + // Same (x + y), so use plusOp/sumOp - less fragile (issue #3326) + void operator()(const MinMax<T>& x, const MinMax<T>& y) const = delete; }; @@ -174,10 +172,8 @@ struct minMaxEqOp return x.add(y); } - MinMax<T>& operator()(MinMax<T>& x, const MinMax<T>& y) const - { - return x.add(y); - } + // Same as (x += y), so use plusEqOp - less fragile (issue #3326) + void operator()(MinMax<T>& x, const MinMax<T>& y) const = delete; }; @@ -234,12 +230,7 @@ inline scalarMinMax minMaxMag(const T& x, const T& y) template<class T> inline scalarMinMax minMaxMag(const MinMax<T>& x, const MinMax<T>& y) { - return - ( - minMaxMag(x) - .add(Foam::mag(y.min())) - .add(Foam::mag(y.max())) - ); + return minMaxMag(x).add(Foam::mag(y.min()), Foam::mag(y.max())); } @@ -247,12 +238,7 @@ inline scalarMinMax minMaxMag(const MinMax<T>& x, const MinMax<T>& y) template<class T1, class T2> inline scalarMinMax minMaxMag(const MinMax<T1>& x, const MinMax<T2>& y) { - return - ( - minMaxMag(x) - .add(Foam::mag(y.min())) - .add(Foam::mag(y.max())) - ); + return minMaxMag(x).add(Foam::mag(y.min()), Foam::mag(y.max())); } @@ -287,12 +273,7 @@ struct minMaxMagEqOp { x = minMaxMag(x); - return - ( - x - .add(Foam::mag(y.min())) - .add(Foam::mag(y.max())) - ); + return x.add(Foam::mag(y.min()), Foam::mag(y.max())); } scalarMinMax& operator()(scalarMinMax& x, const UList<T>& y) const @@ -321,7 +302,7 @@ inline MinMax<T> operator+(const MinMax<T>& x, const MinMax<T>& y) //- Multiply range by scalar factor template<class T> -inline MinMax<T> operator*(const MinMax<T>& x, const scalar& s) +inline MinMax<T> operator*(const MinMax<T>& x, scalar s) { return MinMax<T>(x.min()*s, x.max()*s); } @@ -329,7 +310,7 @@ inline MinMax<T> operator*(const MinMax<T>& x, const scalar& s) //- Divide range by scalar factor template<class T> -inline MinMax<T> operator/(const MinMax<T>& x, const scalar& s) +inline MinMax<T> operator/(const MinMax<T>& x, scalar s) { return MinMax<T>(x.min()/s, x.max()/s); } diff --git a/src/finiteArea/faMesh/faMeshTools/faMeshToolsChecks.C b/src/finiteArea/faMesh/faMeshTools/faMeshToolsChecks.C index 7664356be93..e166a0bdd25 100644 --- a/src/finiteArea/faMesh/faMeshTools/faMeshToolsChecks.C +++ b/src/finiteArea/faMesh/faMeshTools/faMeshToolsChecks.C @@ -161,14 +161,14 @@ void Foam::faMeshTools::printMeshChecks scalarMinMax limit(minMax(mesh.magLe().primitiveField())); // Include processor boundaries into 'internal' edges - if (Pstream::parRun()) + if (UPstream::parRun()) { for (label patchi = nNonProcessor; patchi < nPatches; ++patchi) { limit.add(minMax(mesh.magLe().boundaryField()[patchi])); } - reduce(limit, minMaxOp<scalar>()); + reduce(limit, plusOp<scalarMinMax>{}); } Info<< "Edge length (internal):" << nl @@ -181,9 +181,9 @@ void Foam::faMeshTools::printMeshChecks limit.add(minMax(mesh.magLe().boundaryField()[patchi])); } - if (Pstream::parRun()) + if (UPstream::parRun()) { - reduce(limit, minMaxOp<scalar>()); + reduce(limit, plusOp<scalarMinMax>{}); } Info<< "Edge length:" << nl diff --git a/src/functionObjects/field/binField/binModels/singleDirectionUniformBin/singleDirectionUniformBin.C b/src/functionObjects/field/binField/binModels/singleDirectionUniformBin/singleDirectionUniformBin.C index 4d66960b97f..6b39e51995b 100644 --- a/src/functionObjects/field/binField/binModels/singleDirectionUniformBin/singleDirectionUniformBin.C +++ b/src/functionObjects/field/binField/binModels/singleDirectionUniformBin/singleDirectionUniformBin.C @@ -75,7 +75,7 @@ void Foam::binModels::singleDirectionUniformBin::initialise() } // Globally consistent - reduce(geomLimits, minMaxOp<scalar>()); + reduce(geomLimits, sumOp<scalarMinMax>()); if (!geomLimits.good()) { diff --git a/src/functionObjects/field/binField/binModels/uniformBin/uniformBin.C b/src/functionObjects/field/binField/binModels/uniformBin/uniformBin.C index dc122d42362..65f9a19643f 100644 --- a/src/functionObjects/field/binField/binModels/uniformBin/uniformBin.C +++ b/src/functionObjects/field/binField/binModels/uniformBin/uniformBin.C @@ -58,9 +58,7 @@ void Foam::binModels::uniformBin::initialise() ); MinMax<vector> limits(pts); - - geomLimits.add(limits.min()); - geomLimits.add(limits.max()); + geomLimits.add(limits.min(), limits.max()); } for (const label zonei : cellZoneIDs_) @@ -72,9 +70,7 @@ void Foam::binModels::uniformBin::initialise() ); MinMax<vector> limits(pts); - - geomLimits.add(limits.min()); - geomLimits.add(limits.max()); + geomLimits.add(limits.min(), limits.max()); } // Globally consistent diff --git a/src/meshTools/AMIInterpolation/GAMG/interfaces/cyclicAMIGAMGInterface/cyclicAMIGAMGInterface.C b/src/meshTools/AMIInterpolation/GAMG/interfaces/cyclicAMIGAMGInterface/cyclicAMIGAMGInterface.C index 69dd64b6380..f2a2d0ebe4d 100644 --- a/src/meshTools/AMIInterpolation/GAMG/interfaces/cyclicAMIGAMGInterface/cyclicAMIGAMGInterface.C +++ b/src/meshTools/AMIInterpolation/GAMG/interfaces/cyclicAMIGAMGInterface/cyclicAMIGAMGInterface.C @@ -180,8 +180,7 @@ Foam::cyclicAMIGAMGInterface::cyclicAMIGAMGInterface const auto& AMI = amiPtr_(); if (debug & 2) { - const auto oldWarnComm = UPstream::warnComm; - UPstream::warnComm = AMI.comm(); + const auto oldWarnComm = UPstream::commWarn(AMI.comm()); const label myRank = UPstream::myProcNo(AMI.comm()); Pout<< "At level:" << fineLevelIndex @@ -297,7 +296,7 @@ Foam::cyclicAMIGAMGInterface::cyclicAMIGAMGInterface } Pout<< "DONE agglomerating at level:" << fineLevelIndex << endl; - UPstream::warnComm = oldWarnComm; + UPstream::commWarn(oldWarnComm); } } } @@ -820,8 +819,7 @@ Foam::cyclicAMIGAMGInterface::cyclicAMIGAMGInterface { const auto& AMI = amiPtr_(); - const auto oldWarnComm = UPstream::warnComm; - UPstream::warnComm = AMI.comm(); + const auto oldWarnComm = UPstream::commWarn(AMI.comm()); const label myRank = UPstream::myProcNo(AMI.comm()); Pout<< "PROCAGGLOMERATED :" @@ -930,7 +928,7 @@ Foam::cyclicAMIGAMGInterface::cyclicAMIGAMGInterface } } Pout<< "DONE PROCAGGLOMERATED" << endl; - UPstream::warnComm = oldWarnComm; + UPstream::commWarn(oldWarnComm); } } } -- GitLab