2018-07-16 17:32 BST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0000993OpenFOAM[All Projects] Bugpublic2018-07-10 11:21
Reporterdkxls 
Assigned Touser2 
PrioritynormalSeverityminorReproducibilityN/A
StatusresolvedResolutionfixed 
PlatformLinux x86_64OSopenSUSEOS Version12.2
Product Version 
Target VersionFixed in Version 
Summary0000993: [BreakupModel]: Inconsistent or no reading of TAB/ETAB coefficients leading to wrong sphere deviation
DescriptionIn the present implementation, the TAB coefficients in BreakupModel are initialized to 0.0 and only read/set to proper values if "solveOscillationEq" is true. In case the coefficients are read, then only ones in the BreakupModel constructor.
This lead to the following issues:

1. The TAB model sets it's coefficients to the ones from the BreakupModel. If "solveOscillationEq" is false, the TAB constructor sets it to true, but coefficients are still 0.0 as the reading takes place in the BreakupModel constructor prior to setting solveOscillationEq to true.

2. The ETAB model reads a own set of coefficients, which could lead to different coefficients in the ETAB and the BreakupModel (and hence in the solveTABEq function). Besides, also here it is possible that the BreakupModel coefficients are not read and left at 0.0 as in the TAB model.

These issues are not exactly critical as they can be circumvented by setting the appropriate switches correctly, but this require the user to exactly know which switches to set where.
I only understood it after examining the source code - the tutorial was rather confusing!
Additional InformationThe oscillation equation should probably be moved from the SprayParcel to the BreakupModel, as it is an integral part of it and requires also the coefficients from there.
Although, the DistortedSphereDragForce drag model (code provided in bug #991) requires the oscillation equation to be solved.
Tagsbreakup, spray
Attached Files
  • patch file icon 0001-Fix-inconsistent-breakup-coefficient-reading.patch (5,035 bytes) 2013-09-04 19:37 -
    diff --git a/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.C b/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.C
    index f714be3..9439fc4 100644
    --- a/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.C
    +++ b/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.C
    @@ -64,27 +64,42 @@ Foam::BreakupModel<CloudType>::BreakupModel
     (
         const dictionary& dict,
         CloudType& owner,
    -    const word& type
    +    const word& type,
    +    const Switch solveOscillationEq = false
     )
     :
         SubModelBase<CloudType>(owner, dict, typeName, type),
    -    solveOscillationEq_(this->coeffDict().lookup("solveOscillationEq")),
    +    solveOscillationEq_(solveOscillationEq),
         y0_(0.0),
         yDot0_(0.0),
    -    TABComega_(0.0),
    -    TABCmu_(0.0),
    -    TABWeCrit_(0.0)
    +    TABComega_(8.0),
    +    TABCmu_(10.0),
    +    TABWeCrit_(12.0)
     {
    +    // Give the option to solve the oscillation equation, even if not required 
    +    // by the breakup model
    +    if (!solveOscillationEq_)
    +    {
    +        this->coeffDict().readIfPresent
    +        (
    +            "solveOscillationEq",
    +            solveOscillationEq_
    +        );
    +    }
    +
    +    // Check for non-default TAB coefficients
         if (solveOscillationEq_)
         {
             const dictionary TABcoeffsDict(dict.subDict("TABCoeffs"));
    -        y0_ = TABcoeffsDict.template lookupOrDefault<scalar>("y0", 0.0);
    -        yDot0_ = TABcoeffsDict.template lookupOrDefault<scalar>("yDot0", 0.0);
    -        TABComega_ =
    -            TABcoeffsDict.template lookupOrDefault<scalar>("Comega", 8.0);
    -        TABCmu_ = TABcoeffsDict.template lookupOrDefault<scalar>("Cmu", 10.0);
    -        TABWeCrit_ =
    -            TABcoeffsDict.template lookupOrDefault<scalar>("WeCrit", 12.0);
    +        Switch defaultCoeffs = TABcoeffsDict.lookup("defaultCoeffs");
    +        if (!defaultCoeffs)
    +        {
    +            TABcoeffsDict.lookup("y0") >> y0_;
    +            TABcoeffsDict.lookup("yDot0") >> yDot0_;
    +            TABcoeffsDict.lookup("Comega") >> TABComega_;
    +            TABcoeffsDict.lookup("Cmu") >> TABCmu_;
    +            TABcoeffsDict.lookup("WeCrit") >> TABWeCrit_;
    +        }
         }
     }
     
    diff --git a/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.H b/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.H
    index e0672ca..09cbe36 100644
    --- a/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.H
    +++ b/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.H
    @@ -97,7 +97,8 @@ public:
             (
                 const dictionary& dict,
                 CloudType& owner,
    -            const word& type
    +            const word& type,
    +            const Switch solveOscillationEq
             );
     
             //- Construct copy
    diff --git a/src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C b/src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C
    index fb48040..7f08e47 100644
    --- a/src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C
    +++ b/src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C
    @@ -34,7 +34,7 @@ Foam::ETAB<CloudType>::ETAB
         CloudType& owner
     )
     :
    -    BreakupModel<CloudType>(dict, owner, typeName),
    +    BreakupModel<CloudType>(dict, owner, typeName, true),
         Cmu_(10.0),
         Comega_(8.0),
         k1_(0.2),
    @@ -55,15 +55,6 @@ Foam::ETAB<CloudType>::ETAB
     
         scalar k21 = k2_/k1_;
         AWe_ = (k21*sqrt(WeTransition_) - 1.0)/pow4(WeTransition_);
    -
    -    if (!BreakupModel<CloudType>::solveOscillationEq_)
    -    {
    -        Info<< "Warning: solveOscillationEq is set to "
    -            << BreakupModel<CloudType>::solveOscillationEq_ << nl
    -            << " Setting it to true in order for the ETAB model to work."
    -            << endl;
    -        BreakupModel<CloudType>::solveOscillationEq_ = true;
    -    }
     }
     
     
    diff --git a/src/lagrangian/spray/submodels/BreakupModel/TAB/TAB.C b/src/lagrangian/spray/submodels/BreakupModel/TAB/TAB.C
    index 0fe325f..53f416f 100644
    --- a/src/lagrangian/spray/submodels/BreakupModel/TAB/TAB.C
    +++ b/src/lagrangian/spray/submodels/BreakupModel/TAB/TAB.C
    @@ -34,7 +34,7 @@ Foam::TAB<CloudType>::TAB
         CloudType& owner
     )
     :
    -    BreakupModel<CloudType>(dict, owner, typeName),
    +    BreakupModel<CloudType>(dict, owner, typeName, true),
         Cmu_(BreakupModel<CloudType>::TABCmu_),
         Comega_(BreakupModel<CloudType>::TABComega_),
         WeCrit_(BreakupModel<CloudType>::TABWeCrit_),
    @@ -52,16 +52,6 @@ Foam::TAB<CloudType>::TAB
                 (1.0 - exp(-xx)*(1.0 + xx + sqr(xx)/2.0 + pow3(xx)/6.0))*rrd100;
         }
     
    -    if (!BreakupModel<CloudType>::solveOscillationEq_)
    -    {
    -        WarningIn("Foam::TAB<CloudType>::TAB(const dictionary&, CloudType&)")
    -            << "solveOscillationEq is set to "
    -            << BreakupModel<CloudType>::solveOscillationEq_ << nl
    -            << " Setting it to true in order for the TAB model to work."
    -            << endl;
    -        BreakupModel<CloudType>::solveOscillationEq_ = true;
    -    }
    -
         if (SMDCalcMethod_ == "method1")
         {
             SMDMethod_ = method1;
    
  • patch file icon 0001-Fix-inconsistent-ETAB-coefficient-reading.patch (1,410 bytes) 2013-09-04 19:37 -
    From c0e01d022e1923d080872c812408e5f78165ebe2 Mon Sep 17 00:00:00 2001
    From: Armin Wehrfritz <armin.wehrfritz@aalto.fi>
    Date: Wed, 4 Sep 2013 21:32:39 +0300
    Subject: [PATCH] Fix inconsistent ETAB coefficient reading
    
    ---
     src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C |    9 +++------
     1 file changed, 3 insertions(+), 6 deletions(-)
    
    diff --git a/src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C b/src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C
    index 7f08e47..24a33e1 100644
    --- a/src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C
    +++ b/src/lagrangian/spray/submodels/BreakupModel/ETAB/ETAB.C
    @@ -35,21 +35,18 @@ Foam::ETAB<CloudType>::ETAB
     )
     :
         BreakupModel<CloudType>(dict, owner, typeName, true),
    -    Cmu_(10.0),
    -    Comega_(8.0),
    +    Cmu_(BreakupModel<CloudType>::TABCmu_),
    +    Comega_(BreakupModel<CloudType>::TABComega_),
         k1_(0.2),
         k2_(0.2),
    -    WeCrit_(12.0),
    +    WeCrit_(BreakupModel<CloudType>::TABWeCrit_),
         WeTransition_(100.0),
         AWe_(0.0)
     {
         if (!this->defaultCoeffs(true))
         {
    -        this->coeffDict().lookup("Cmu") >> Cmu_;
    -        this->coeffDict().lookup("Comega") >> Comega_;
             this->coeffDict().lookup("k1") >> k1_;
             this->coeffDict().lookup("k2") >> k2_;
    -        this->coeffDict().lookup("WeCrit") >> WeCrit_;
             this->coeffDict().lookup("WeTransition") >> WeTransition_;
         }
     
    -- 
    1.7.10.4
    
    
  • patch file icon 0001-Enhance-TAB-coefficient-reading-and-add-enableOscill.patch (4,043 bytes) 2013-09-04 20:38 -
    From 48872ed9df56504a8b3a60eff752ff758cc75049 Mon Sep 17 00:00:00 2001
    From: Armin Wehrfritz <armin.wehrfritz@aalto.fi>
    Date: Wed, 4 Sep 2013 22:26:42 +0300
    Subject: [PATCH] Enhance TAB coefficient reading and add
     enableOscillationEq()
    
    ---
     .../BreakupModel/BreakupModel/BreakupModel.C       |   54 ++++++++++++++++----
     .../BreakupModel/BreakupModel/BreakupModel.H       |   10 ++++
     2 files changed, 54 insertions(+), 10 deletions(-)
    
    diff --git a/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.C b/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.C
    index 9439fc4..f5fa020 100644
    --- a/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.C
    +++ b/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.C
    @@ -25,6 +25,30 @@ License
     
     #include "BreakupModel.H"
     
    +// * * * * * * * * * * * * Private Member Functions  * * * * * * * * * * * * //
    +
    +template<class CloudType>
    +void Foam::BreakupModel<CloudType>::readTABCoeffs(const bool printMsg)
    +{
    +    const dictionary TABcoeffsDict(this->dict().subDict("TABCoeffs"));
    +    bool def = TABcoeffsDict.lookupOrDefault<bool>("defaultCoeffs", false);
    +    if (!def)
    +    {
    +        TABcoeffsDict.lookup("y0") >> y0_;
    +        TABcoeffsDict.lookup("yDot0") >> yDot0_;
    +        TABcoeffsDict.lookup("Comega") >> TABComega_;
    +        TABcoeffsDict.lookup("Cmu") >> TABCmu_;
    +        TABcoeffsDict.lookup("WeCrit") >> TABWeCrit_;
    +    }
    +    if (printMsg && def)
    +    {
    +        Info<< incrIndent;
    +        Info<< indent << "Employing default TAB coefficients" << endl;
    +        Info<< decrIndent;
    +    }
    +}
    +
    +
     // * * * * * * * * * * * * * * * * Constructors  * * * * * * * * * * * * * * //
     
     template<class CloudType>
    @@ -90,16 +114,7 @@ Foam::BreakupModel<CloudType>::BreakupModel
         // Check for non-default TAB coefficients
         if (solveOscillationEq_)
         {
    -        const dictionary TABcoeffsDict(dict.subDict("TABCoeffs"));
    -        Switch defaultCoeffs = TABcoeffsDict.lookup("defaultCoeffs");
    -        if (!defaultCoeffs)
    -        {
    -            TABcoeffsDict.lookup("y0") >> y0_;
    -            TABcoeffsDict.lookup("yDot0") >> yDot0_;
    -            TABcoeffsDict.lookup("Comega") >> TABComega_;
    -            TABcoeffsDict.lookup("Cmu") >> TABCmu_;
    -            TABcoeffsDict.lookup("WeCrit") >> TABWeCrit_;
    -        }
    +        readTABCoeffs(true);
         }
     }
     
    @@ -114,6 +129,25 @@ Foam::BreakupModel<CloudType>::~BreakupModel()
     // * * * * * * * * * * * * * * * Member Functions  * * * * * * * * * * * * * //
     
     template<class CloudType>
    +void Foam::BreakupModel<CloudType>::enableOscillationEq(const bool printMsg)
    +{
    +    if (!solveOscillationEq_)
    +    {
    +        solveOscillationEq_ = true;
    +
    +        if (printMsg)
    +        {
    +            Info<< incrIndent;
    +            Info<< indent << "Enable oscillation equation solution" << endl;
    +            Info<< decrIndent;
    +        }
    +
    +        readTABCoeffs(printMsg);
    +    }
    +}
    +
    +
    +template<class CloudType>
     bool Foam::BreakupModel<CloudType>::update
     (
         const scalar dt,
    diff --git a/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.H b/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.H
    index 09cbe36..ac54b73 100644
    --- a/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.H
    +++ b/src/lagrangian/spray/submodels/BreakupModel/BreakupModel/BreakupModel.H
    @@ -55,6 +55,12 @@ class BreakupModel
         public SubModelBase<CloudType>
     {
     
    +    // Private Member Functions
    +
    +        //- Read TAB coefficients
    +        void readTABCoeffs(const bool printMsg);
    +
    +
     protected:
     
         // Protected data
    @@ -161,6 +167,10 @@ public:
     
         // Member Functions
     
    +        //- Enable the solution of the oscillation equation and check that the 
    +        //  TAB coefficients are set correctly
    +        void enableOscillationEq(const bool printMsg);
    +
             //- Update the parcel properties and return true if a child parcel
             //  should be added
             virtual bool update
    -- 
    1.7.10.4
    
    
  • ? file icon BreakupModel.H (6,983 bytes) 2013-09-04 20:45
  • c file icon BreakupModel.C (5,451 bytes) 2013-09-04 20:45
  • c file icon ETAB.C (5,699 bytes) 2013-09-04 20:45
  • c file icon TAB.C (6,350 bytes) 2013-09-04 20:45

-Relationships
+Relationships

-Notes

~0002461

dkxls (reporter)

Last edited: 2013-09-05 00:24

View 2 revisions

The patches I uploaded fix the two issues and should guarantee consistent coefficients for the breakup models.

~0002462

dkxls (reporter)

The last patch enhances the TAB coefficient reading and adds a function 'enableOscillationEq()' that makes it possible to enable the solution of the oscillation equation and checks that the coefficients are set correctly.

The last patch needs to be applied atop the other two patches.
Sorry for the bit messy patch arrangement, I therefor also uploaded all changed source files. Hope this improves the readability.

The code is tested and works as expected.

~0002467

user2

Thanks for the report - fixed by commit 1064355
+Notes

-Issue History
Date Modified Username Field Change
2013-09-04 19:16 dkxls New Issue
2013-09-04 19:16 dkxls Tag Attached: breakup
2013-09-04 19:16 dkxls Tag Attached: spray
2013-09-04 19:16 dkxls Tag Attached: ETAB
2013-09-04 19:16 dkxls Tag Attached: TAB
2013-09-04 19:37 dkxls File Added: 0001-Fix-inconsistent-breakup-coefficient-reading.patch
2013-09-04 19:37 dkxls File Added: 0001-Fix-inconsistent-ETAB-coefficient-reading.patch
2013-09-04 19:46 dkxls Note Added: 0002461
2013-09-04 20:38 dkxls File Added: 0001-Enhance-TAB-coefficient-reading-and-add-enableOscill.patch
2013-09-04 20:44 dkxls Note Added: 0002462
2013-09-04 20:45 dkxls File Added: BreakupModel.H
2013-09-04 20:45 dkxls File Added: BreakupModel.C
2013-09-04 20:45 dkxls File Added: ETAB.C
2013-09-04 20:45 dkxls File Added: TAB.C
2013-09-05 00:24 dkxls Note Edited: 0002461 View Revisions
2013-09-05 18:19 user2 Note Added: 0002467
2013-09-05 18:19 user2 Status new => resolved
2013-09-05 18:19 user2 Fixed in Version => 2.2.x
2013-09-05 18:19 user2 Resolution open => fixed
2013-09-05 18:19 user2 Assigned To => user2
2018-07-10 11:18 administrator Tag Detached: ETAB
2018-07-10 11:21 administrator Tag Detached: TAB
+Issue History