2017-12-15 23:42 GMT

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002775OpenFOAM[All Projects] Bugpublic2017-12-12 12:23
Reporterrikhouthuys 
Assigned Tohenry 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionunable to reproduce 
PlatformGNU/LinuxOSUbuntuOS Version14.04
Product Version5.0 
Target VersionFixed in Version 
Summary0002775: pimpleControl::loop() execute additional iteration after convergence criteria satisfied
DescriptionLooking at the source code of pimpleControl::loop()

     if (converged_ || criteriaSatisfied())
     {
         if (converged_)
         {
             Info<< algorithmName_ << ": converged in " << corr_ - 1
                 << " iterations" << endl;
 
             mesh_.data::remove("finalIteration");
             corr_ = 0;
             converged_ = false;
 
             completed = true;
         }
         else
         {
     Info<< algorithmName_ << ": iteration " << corr_ << endl;
     storePrevIterFields();

     mesh_.data::add("finalIteration", true);
             converged_ = true;
         }
     }
So, if criteria satisfied but converged_ flag is "false", loop() will return "true" i.e. the current iteration will be executed although the criteria has been satisfied before this iteration, i.e. this iteration is redundant.

In the next iteration: converged_ flag becomes"true" then loop() will return "false" and PIMPLE loop for this time step will be terminated
TagspimpleControl
Attached Files

-Relationships
+Relationships

-Notes

~0009094

henry (manager)

It is not clear what change you are proposing or what the implications are. Have you tested any changes to the logic?

~0009106

rikhouthuys (reporter)

I'm proposing two changes to this function:
bool Foam::pimpleControl::loop()
 {
     read();
 
     corr_++;
 
     if (corr_ == nCorrPIMPLE_ + 1)
     {
         if ((!residualControl_.empty()) && (nCorrPIMPLE_ != 1))
         {
             Info<< algorithmName_ << ": not converged within "
                 << nCorrPIMPLE_ << " iterations" << endl;
         }
 
         corr_ = 0;
         mesh_.data::remove("finalIteration");
         converged_ = false; // 1st change: useful in case criteria satisfied in last outer iteration
         return false;
     }
 
     bool completed = false;
 if (converged_ || criteriaSatisfied())
 {
     if (converged_)
     {
         Info<< algorithmName_ << ": converged in " << corr_ - 1
             << " iterations" << endl;

         mesh_.data::remove("finalIteration");
         corr_ = 0;
         converged_ = false;

         completed = true;
     }
     else
     {
     Info<< algorithmName_ << ": iteration " << corr_ << endl;
     storePrevIterFields();

     mesh_.data::add("finalIteration", true);
             converged_ = true;
             completed = true; // 2nd change to avoid redundant iteration
         }
     }
     else
     {
         if (finalIter())
         {
             mesh_.data::add("finalIteration", true);
         }
 
         if (corr_ <= nCorrPIMPLE_)
         {
             Info<< algorithmName_ << ": iteration " << corr_ << endl;
             storePrevIterFields();
             completed = false;
         }
     }
 
     return !completed;
 }

~0009108

henry (manager)

What testing has been done on these changes? Do they ensure that fields are written for the converged time-step?

~0009109

henry (manager)

Could you also explain the rationale for the change and what it achieves?

~0009140

henry (manager)

The changes you propose cause the

tutorials/compressible/rhoPimpleFoam/RAS/angledDuct

case to behave very oddly:

Courant Number mean: 6525.72 max: 11819.3
Time = 8

PIMPLE: converged in 39 iterations
ExecutionTime = 31.2 s ClockTime = 32 s

Courant Number mean: 6525.72 max: 11819.3
Time = 9

PIMPLE: iteration 1
smoothSolver: Solving for Ux, Initial residual = 4.4255e-06, Final residual = 2.13643e-07, No Iterations 1
smoothSolver: Solving for Uy, Initial residual = 4.40234e-06, Final residual = 2.06258e-07, No Iterations 1
smoothSolver: Solving for Uz, Initial residual = 9.6979e-05, Final residual = 3.35768e-06, No Iterations 1
smoothSolver: Solving for e, Initial residual = 1.4807e-05, Final residual = 1.33455e-06, No Iterations 1
DICPCG: Solving for p, Initial residual = 3.15685e-06, Final residual = 9.15688e-08, No Iterations 25
diagonal: Solving for rho, Initial residual = 0, Final residual = 0, No Iterations 0
time step continuity errors : sum local = 6.165e-05, global = 3.27513e-06, cumulative = 0.0035585
smoothSolver: Solving for epsilon, Initial residual = 8.38565e-07, Final residual = 8.38565e-07, No Iterations 0
smoothSolver: Solving for k, Initial residual = 3.43419e-06, Final residual = 7.02118e-07, No Iterations 1
PIMPLE: iteration 2
ExecutionTime = 31.27 s ClockTime = 32 s

Courant Number mean: 6525.72 max: 11819.3
Time = 10

PIMPLE: converged in 2 iterations
ExecutionTime = 31.4 s ClockTime = 32 s

End

Have you testing this case? Do you find the same? What other tests have you run to demonstrate the need and correctness of the change?

~0009148

henry (manager)

Orphaned report.
+Notes

-Issue History
Date Modified Username Field Change
2017-11-28 22:21 rikhouthuys New Issue
2017-11-28 22:21 rikhouthuys Tag Attached: pimpleControl
2017-11-28 23:32 henry Note Added: 0009094
2017-11-29 12:52 rikhouthuys Note Added: 0009106
2017-11-29 13:51 henry Note Added: 0009108
2017-11-29 13:52 henry Note Added: 0009109
2017-12-08 13:12 henry Note Added: 0009140
2017-12-12 12:23 henry Assigned To => henry
2017-12-12 12:23 henry Status new => closed
2017-12-12 12:23 henry Resolution open => unable to reproduce
2017-12-12 12:23 henry Note Added: 0009148
+Issue History