View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002775OpenFOAMBugpublic2017-11-28 22:212017-12-12 12:23
Reporterrikhouthuys Assigned Tohenry  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionunable to reproduce 
PlatformGNU/LinuxOSUbuntuOS Version14.04
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

Activities

henry

2017-11-28 23:32

manager   ~0009094

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

rikhouthuys

2017-11-29 12:52

reporter   ~0009106

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;
 }

henry

2017-11-29 13:51

manager   ~0009108

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

henry

2017-11-29 13:52

manager   ~0009109

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

henry

2017-12-08 13:12

manager   ~0009140

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?

henry

2017-12-12 12:23

manager   ~0009148

Orphaned report.

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