From 97be8fc7676e80a587377bf1163c5b4f7507747f Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Thu, 24 Sep 2020 18:36:55 +0200
Subject: [PATCH] ENH: add finer control to the expected expression types

- adds "future-proofing" for derived expression boundary conditions
  by moving potential failure modes into the base class.
---
 .../fields/base/patchExprFieldBase.C          | 158 ++++++++++--------
 .../fields/base/patchExprFieldBase.H          |  51 ++++--
 .../exprFixedValueFvPatchField.C              |  17 +-
 .../exprFixedValueFvPatchField.H              |  12 +-
 .../fvPatchFields/exprMixedFvPatchField.C     |   8 +-
 .../fvPatchFields/exprMixedFvPatchField.H     |  20 ++-
 .../exprValuePointPatchField.C                |  41 +++--
 .../exprValuePointPatchField.H                |   7 +-
 8 files changed, 184 insertions(+), 130 deletions(-)

diff --git a/src/finiteVolume/expressions/fields/base/patchExprFieldBase.C b/src/finiteVolume/expressions/fields/base/patchExprFieldBase.C
index 42a52693591..64176eb47c1 100644
--- a/src/finiteVolume/expressions/fields/base/patchExprFieldBase.C
+++ b/src/finiteVolume/expressions/fields/base/patchExprFieldBase.C
@@ -33,115 +33,133 @@ License
 #include "pointMesh.H"
 #include "stringOps.H"
 
-// * * * * * * * * * * * * * * * * Constructors  * * * * * * * * * * * * * * //
-
-Foam::expressions::patchExprFieldBase::patchExprFieldBase()
-:
-    patchExprFieldBase(false)
-{}
+// * * * * * * * * * * * * * Private Member Functions  * * * * * * * * * * * //
 
-
-Foam::expressions::patchExprFieldBase::patchExprFieldBase
-(
-    bool allowGradient
-)
-:
-    debug_(false),
-    allowGradient_(allowGradient),
-    evalOnConstruct_(false),
-    valueExpr_(),
-    gradExpr_(),
-    fracExpr_()
-{}
-
-
-Foam::expressions::patchExprFieldBase::patchExprFieldBase
+void Foam::expressions::patchExprFieldBase::readExpressions
 (
     const dictionary& dict,
-    bool allowGradient,
+    enum expectedTypes expectedType,
     bool isPointVal
 )
-:
-    debug_(dict.getOrDefault("debug", false)),
-    allowGradient_(allowGradient),
-    evalOnConstruct_(dict.getOrDefault("evalOnConstruct", false)),
-    valueExpr_(),
-    gradExpr_(),
-    fracExpr_()
 {
     if (debug_)
     {
         Info<< "Expression BC with " << dict << nl;
     }
 
-    string expr;
+    valueExpr_.clear();
+    gradExpr_.clear();
+    fracExpr_.clear();
+
+    string exprValue, exprGrad, exprFrac;
+    bool evalValue = false, evalGrad = false, evalFrac = false;
 
-    if (dict.readIfPresent("valueExpr", expr))
+    if (expectedTypes::VALUE_TYPE == expectedType)
+    {
+        // Mandatory
+        evalValue = dict.readEntry("valueExpr", exprValue);
+    }
+    else if (expectedTypes::GRADIENT_TYPE == expectedType)
     {
-        valueExpr_ = expressions::exprString(expr, dict);
+        // Mandatory
+        evalGrad = dict.readEntry("gradientExpr", exprGrad);
     }
     else
     {
-        // No value expression - same as Zero
+        // MIXED_TYPE
+        evalValue = dict.readIfPresent("valueExpr", exprValue);
+        evalGrad = dict.readIfPresent("gradientExpr", exprGrad);
+
+        if (!evalValue && !evalGrad)
+        {
+            FatalIOErrorInFunction(dict)
+                << "Entries 'valueExpr' and 'gradientExpr' "
+                   "(mixed-conditon) not found in dictionary "
+                << dict.name() << nl
+                << exit(FatalIOError);
+        }
+
         if (debug_)
         {
-            Info<< "No valueExpr" << nl;
+            if (!evalValue)
+            {
+                Info<< "Mixed with no valueExpr" << nl;
+            }
+            if (!evalGrad)
+            {
+                Info<< "Mixed with no gradientExpr" << nl;
+            }
         }
     }
 
-    if (allowGradient)
+
+    // When both value/gradient specified (ie, mixed) expect a fraction
+    // - if missing, defer treatment to inherited BC
+
+    if (evalValue && evalGrad && dict.readIfPresent("fractionExpr", exprFrac))
     {
-        if (dict.readIfPresent("gradientExpr", expr))
+        stringOps::inplaceTrim(exprFrac);
+
+        if (exprFrac == "0" || exprFrac == "1")
         {
-            gradExpr_ = expressions::exprString(expr, dict);
+            // Special cases, handled with more efficiency
+            fracExpr_ = exprFrac;
         }
-        else
+        else if (!exprFrac.empty())
         {
-            // No gradient expression - same as Zero
-            if (debug_)
+            evalFrac = true;
+            if (isPointVal)
             {
-                Info<< "No gradientExpr" << nl;
+                exprFrac = "toPoint(" + exprFrac + ")";
             }
         }
+    }
 
-        if (dict.readIfPresent("fractionExpr", expr))
-        {
-            stringOps::inplaceTrim(expr);
-
-            if (expr == "0" || expr == "1")
-            {
-                // Special cases, handled with more efficiency
-                fracExpr_ = expr;
-            }
-            else if (!expr.empty())
-            {
-                if (isPointVal)
-                {
-                    expr = "toPoint(" + expr + ")";
-                }
 
-                fracExpr_ = expressions::exprString(expr, dict);
-            }
-        }
+    // Expansions
 
-        // No fraction expression? - defer treatment to inherited BC
-        // Mixed BC may elect to simply ignore gradient expression
+    if (evalValue)
+    {
+        valueExpr_ = expressions::exprString(exprValue, dict);
+    }
+    if (evalGrad)
+    {
+        gradExpr_ = expressions::exprString(exprGrad, dict);
+    }
+    if (evalFrac)
+    {
+        fracExpr_ = expressions::exprString(exprFrac, dict);
     }
 }
 
 
+// * * * * * * * * * * * * * * * * Constructors  * * * * * * * * * * * * * * //
+
+Foam::expressions::patchExprFieldBase::patchExprFieldBase()
+:
+    debug_(false),
+    evalOnConstruct_(false),
+    valueExpr_(),
+    gradExpr_(),
+    fracExpr_()
+{}
+
+
 Foam::expressions::patchExprFieldBase::patchExprFieldBase
 (
-    const patchExprFieldBase& rhs
+    const dictionary& dict,
+    enum expectedTypes expectedType,
+    bool isPointVal
 )
 :
-    debug_(rhs.debug_),
-    allowGradient_(rhs.allowGradient_),
-    evalOnConstruct_(rhs.evalOnConstruct_),
-    valueExpr_(rhs.valueExpr_),
-    gradExpr_(rhs.gradExpr_),
-    fracExpr_(rhs.fracExpr_)
-{}
+    debug_(dict.getOrDefault("debug", false)),
+    evalOnConstruct_(dict.getOrDefault("evalOnConstruct", false)),
+    valueExpr_(),
+    gradExpr_(),
+    fracExpr_()
+{
+    readExpressions(dict, expectedType, isPointVal);
+}
 
 
 // * * * * * * * * * * * * * * * Member Functions  * * * * * * * * * * * * * //
diff --git a/src/finiteVolume/expressions/fields/base/patchExprFieldBase.H b/src/finiteVolume/expressions/fields/base/patchExprFieldBase.H
index 67f3d075955..bb15e768e2e 100644
--- a/src/finiteVolume/expressions/fields/base/patchExprFieldBase.H
+++ b/src/finiteVolume/expressions/fields/base/patchExprFieldBase.H
@@ -29,14 +29,14 @@ Class
 
 Description
     Base class for managing patches with expressions.
-    The expected input supports values, gradients and mixed conditions
+    The expected input supports value, gradient and mixed conditions.
 
 Usage
     \table
         Property     | Description                          | Required | Default
-        valueExpr    | expression for fixed value           | no  | 0
-        gradientExpr | expression for patch normal gradient | no  | 0
-        fractionExpr | expression for value fraction weight | no  | 1
+        valueExpr    | expression for uniformValue          | partly  | 0
+        gradientExpr | expression for uniformGradient       | partly  | 0
+        fractionExpr | expression for valueFraction         | partly  | depends
     \endtable
 
 SourceFiles
@@ -54,12 +54,6 @@ SourceFiles
 
 namespace Foam
 {
-
-// Forward Declarations
-class facePointPatch;
-class fvPatch;
-class polyPatch;
-
 namespace expressions
 {
 
@@ -71,10 +65,21 @@ class patchExprFieldBase
 {
 protected:
 
+    // Protected Types
+
+        //- Enumeration of expected expressions
+        enum expectedTypes
+        {
+            VALUE_TYPE = 1,
+            GRADIENT_TYPE = 2,
+            MIXED_TYPE = 3
+        };
+
+
     // Protected Data
 
+        //- Add debugging
         bool debug_;
-        bool allowGradient_;
 
         //- Slightly dodgy concept here
         bool evalOnConstruct_;
@@ -85,27 +90,37 @@ protected:
         expressions::exprString fracExpr_;
 
 
+private:
+
+    // Private Member Functions
+
+        //- Read expressions from dictionary
+        void readExpressions
+        (
+            const dictionary& dict,
+            enum expectedTypes expectedType,
+            bool isPointVal = false
+        );
+
+
 public:
 
+    // Generated Methods: copy construct, copy assignment
+
+
     // Constructors
 
         //- Default construct
         patchExprFieldBase();
 
-        //- Construct with specified gradient handling
-        explicit patchExprFieldBase(bool allowGradient);
-
         //- Construct from dictionary
         explicit patchExprFieldBase
         (
             const dictionary& dict,
-            bool allowGradient = false,
+            enum expectedTypes expectedType = expectedTypes::VALUE_TYPE,
             bool isPointVal = false
         );
 
-        //- Copy constructor
-        patchExprFieldBase(const patchExprFieldBase& rhs);
-
 
     // Member Functions
 
diff --git a/src/finiteVolume/expressions/fields/fvPatchFields/exprFixedValueFvPatchField.C b/src/finiteVolume/expressions/fields/fvPatchFields/exprFixedValueFvPatchField.C
index d5820ec4820..a7d22e0b98d 100644
--- a/src/finiteVolume/expressions/fields/fvPatchFields/exprFixedValueFvPatchField.C
+++ b/src/finiteVolume/expressions/fields/fvPatchFields/exprFixedValueFvPatchField.C
@@ -50,7 +50,7 @@ Foam::exprFixedValueFvPatchField<Type>::exprFixedValueFvPatchField
 )
 :
     fixedValueFvPatchField<Type>(p, iF),
-    expressions::patchExprFieldBase(false),
+    expressions::patchExprFieldBase(),
     driver_(this->patch())
 {}
 
@@ -83,7 +83,11 @@ Foam::exprFixedValueFvPatchField<Type>::exprFixedValueFvPatchField
 )
 :
     fixedValueFvPatchField<Type>(p, iF),
-    expressions::patchExprFieldBase(dict),
+    expressions::patchExprFieldBase
+    (
+        dict,
+        expressions::patchExprFieldBase::expectedTypes::VALUE_TYPE
+    ),
     driver_(this->patch(), dict)
 {
     setDebug();
@@ -187,14 +191,7 @@ void Foam::exprFixedValueFvPatchField<Type>::updateCoeffs()
 
         if (evalValue)
         {
-            tmp<Field<Type>> tresult(driver_.evaluate<Type>(this->valueExpr_));
-
-            if (debug)
-            {
-                Info<< "Evaluated: " << tresult();
-            }
-
-            (*this) == tresult;
+            (*this) == driver_.evaluate<Type>(this->valueExpr_);
         }
         else
         {
diff --git a/src/finiteVolume/expressions/fields/fvPatchFields/exprFixedValueFvPatchField.H b/src/finiteVolume/expressions/fields/fvPatchFields/exprFixedValueFvPatchField.H
index a1282f9542a..5ef74552ed2 100644
--- a/src/finiteVolume/expressions/fields/fvPatchFields/exprFixedValueFvPatchField.H
+++ b/src/finiteVolume/expressions/fields/fvPatchFields/exprFixedValueFvPatchField.H
@@ -5,8 +5,7 @@
     \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-    Original code Copyright (C) 2009-2018 Bernhard Gschaider
-    Copyright (C) 2019 OpenCFD Ltd.
+    Copyright (C) 2019-2020 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -34,9 +33,13 @@ Usage
     \table
         Property     | Description                          | Required | Default
         value        | fixed value                          | yes |
-        valueExpr    | expression for fixed value           | yes |
+        valueExpr    | expression for uniformValue          | yes |
     \endtable
 
+Note
+    Can also just use uniformFixedValueFvPatchField with an expression
+    for the PatchFunction1.
+
 SourceFiles
     exprFixedValueFvPatchField.C
 
@@ -102,8 +105,7 @@ public:
             const bool valueRequired=true
         );
 
-        //- Construct by mapping given exprFixedValueFvPatchField
-        //- onto a new patch
+        //- Construct by mapping onto a new patch
         exprFixedValueFvPatchField
         (
             const exprFixedValueFvPatchField<Type>&,
diff --git a/src/finiteVolume/expressions/fields/fvPatchFields/exprMixedFvPatchField.C b/src/finiteVolume/expressions/fields/fvPatchFields/exprMixedFvPatchField.C
index e29aafc3877..e16d95ab671 100644
--- a/src/finiteVolume/expressions/fields/fvPatchFields/exprMixedFvPatchField.C
+++ b/src/finiteVolume/expressions/fields/fvPatchFields/exprMixedFvPatchField.C
@@ -49,7 +49,7 @@ Foam::exprMixedFvPatchField<Type>::exprMixedFvPatchField
 )
 :
     mixedFvPatchField<Type>(p, iF),
-    expressions::patchExprFieldBase(true),  // allowGradient
+    expressions::patchExprFieldBase(),
     driver_(this->patch())
 {
     this->refValue() = Zero;
@@ -85,7 +85,11 @@ Foam::exprMixedFvPatchField<Type>::exprMixedFvPatchField
 )
 :
     mixedFvPatchField<Type>(p, iF),
-    expressions::patchExprFieldBase(dict, true),
+    expressions::patchExprFieldBase
+    (
+        dict,
+        expressions::patchExprFieldBase::expectedTypes::MIXED_TYPE
+    ),
     driver_(this->patch(), dict)
 {
     setDebug();
diff --git a/src/finiteVolume/expressions/fields/fvPatchFields/exprMixedFvPatchField.H b/src/finiteVolume/expressions/fields/fvPatchFields/exprMixedFvPatchField.H
index 1c43ce12706..0dc8ce7dcef 100644
--- a/src/finiteVolume/expressions/fields/fvPatchFields/exprMixedFvPatchField.H
+++ b/src/finiteVolume/expressions/fields/fvPatchFields/exprMixedFvPatchField.H
@@ -5,8 +5,7 @@
     \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-    Original code Copyright (C) 2009-2018 Bernhard Gschaider
-    Copyright (C) 2019 OpenCFD Ltd.
+    Copyright (C) 2019-2020 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -33,11 +32,20 @@ Description
 Usage
     \table
         Property     | Description                          | Required | Default
-        valueExpr    | expression for fixed value           | no  | 0
-        gradientExpr | expression for patch normal gradient | no  | 0
-        fractionExpr | expression for value weighting       | no  | 1
+        valueExpr    | expression for uniformValue          | partly  | 0
+        gradientExpr | expression for uniformGradient       | partly  | 0
+        fractionExpr | expression for valueFraction         | partly  | depends
     \endtable
 
+Note
+    For fixed-value boundary conditions, can also just use
+    uniformFixedValueFvPatchField with an expression for the
+    PatchFunction1, or a exprMixedFvPatchField.
+
+    For gradient boundary conditions, can also just use
+    uniformFixedGradientFvPatchField with an expression for the
+    PatchFunction1.
+
 SourceFiles
     exprMixedFvPatchField.C
 
@@ -102,7 +110,7 @@ public:
             const dictionary& dict
         );
 
-        //- Construct by mapping given exprMixedFvPatchField onto a new patch
+        //- Construct by mapping onto a new patch
         exprMixedFvPatchField
         (
             const exprMixedFvPatchField<Type>&,
diff --git a/src/finiteVolume/expressions/fields/pointPatchFields/exprValuePointPatchField.C b/src/finiteVolume/expressions/fields/pointPatchFields/exprValuePointPatchField.C
index 252a7a63506..1ba5e16d834 100644
--- a/src/finiteVolume/expressions/fields/pointPatchFields/exprValuePointPatchField.C
+++ b/src/finiteVolume/expressions/fields/pointPatchFields/exprValuePointPatchField.C
@@ -41,7 +41,7 @@ Foam::exprValuePointPatchField<Type>::exprValuePointPatchField
 )
 :
     valuePointPatchField<Type>(p, iF),
-    expressions::patchExprFieldBase(false),
+    expressions::patchExprFieldBase(),
     driver_
     (
         fvPatch::lookupPatch
@@ -83,7 +83,12 @@ Foam::exprValuePointPatchField<Type>::exprValuePointPatchField
 )
 :
     valuePointPatchField<Type>(p, iF),
-    expressions::patchExprFieldBase(dict, false, true),
+    expressions::patchExprFieldBase
+    (
+        dict,
+        expressions::patchExprFieldBase::expectedTypes::VALUE_TYPE,
+        true // pointValue
+    ),
     driver_
     (
         fvPatch::lookupPatch
@@ -93,7 +98,7 @@ Foam::exprValuePointPatchField<Type>::exprValuePointPatchField
         dict
     )
 {
-    // Basic sanity
+    // Require valueExpr
     if (this->valueExpr_.empty())
     {
         FatalIOErrorInFunction(dict)
@@ -101,6 +106,7 @@ Foam::exprValuePointPatchField<Type>::exprValuePointPatchField
             << exit(FatalIOError);
     }
 
+
     driver_.readDict(dict);
 
     if (dict.found("value"))
@@ -171,34 +177,39 @@ Foam::exprValuePointPatchField<Type>::exprValuePointPatchField
 template<class Type>
 void Foam::exprValuePointPatchField<Type>::updateCoeffs()
 {
+    if (this->updated())
+    {
+        return;
+    }
+
     if (debug)
     {
         InfoInFunction
             << "Value: " << this->valueExpr_ << nl
             << "Variables: ";
-        driver_.writeVariableStrings(Info)  << endl;
+        driver_.writeVariableStrings(Info) << nl;
+        Info<< "... updating" << endl;
     }
 
-    if (this->updated())
-    {
-        return;
-    }
 
     // Expression evaluation
     {
+        bool evalValue = (!this->valueExpr_.empty() && this->valueExpr_ != "0");
+
+
         driver_.clearVariables();
 
-        if (this->valueExpr_.empty())
-        {
-            (*this) == Zero;
-        }
-        else
+        if (evalValue)
         {
             Field<Type>::operator=
             (
                 driver_.evaluate<Type>(this->valueExpr_, true)
             );
         }
+        else
+        {
+            (*this) == Zero;
+        }
     }
 
     valuePointPatchField<Type>::updateCoeffs();
@@ -211,9 +222,9 @@ void Foam::exprValuePointPatchField<Type>::write(Ostream& os) const
     valuePointPatchField<Type>::write(os);
     expressions::patchExprFieldBase::write(os);
 
-    this->writeEntry("value",os);
+    this->writeEntry("value", os);
 
-    driver_.writeCommon(os,this->debug_ || debug);
+    driver_.writeCommon(os, this->debug_ || debug);
 }
 
 
diff --git a/src/finiteVolume/expressions/fields/pointPatchFields/exprValuePointPatchField.H b/src/finiteVolume/expressions/fields/pointPatchFields/exprValuePointPatchField.H
index 8cf146ec3ae..3d6105d6fc7 100644
--- a/src/finiteVolume/expressions/fields/pointPatchFields/exprValuePointPatchField.H
+++ b/src/finiteVolume/expressions/fields/pointPatchFields/exprValuePointPatchField.H
@@ -5,8 +5,7 @@
     \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-    Original code Copyright (C) 2010-2018 Bernhard Gschaider
-    Copyright (C) 2019 OpenCFD Ltd.
+    Copyright (C) 2019-2020 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -34,7 +33,7 @@ Usage
     \table
         Property     | Description                          | Required | Default
         value        | fixed value                          | yes |
-        valueExpr    | expression for fixed value           | yes |
+        valueExpr    | expression for uniformValue          | yes |
     \endtable
 
 SourceFiles
@@ -95,7 +94,7 @@ public:
             const dictionary&
         );
 
-        //- Construct by mapping given patchField<Type> onto a new patch
+        //- Construct by mapping onto a new patch
         exprValuePointPatchField
         (
             const exprValuePointPatchField<Type>&,
-- 
GitLab