[petsc-dev] large bug in KSPSolve_Chebyshev() affects all multigrid solvers; all core developers including Mark please read this

[Smith, Barry F.]

   Mark,

      Thanks for the explanation, I totally missed the 

> if (amatid != cheb->amatid || pmatid != cheb->pmatid || amatstate != cheb->amatstate || pmatstate != cheb->pmatstate) {

code that insures the computation of the eigenvalues is only once on each new matrix.

Sorry for the panic

   Barry


> On Sep 20, 2018, at 5:00 AM, Mark Adams <mfadams at lbl.gov> wrote:
> 
> 
> 
> On Wed, Sep 19, 2018 at 7:44 PM Smith, Barry F. <bsmith at mcs.anl.gov> wrote:
> 
>     Look at the code in KSPSolve_Chebyshev(). 
> 
> Problem 1) VERY MAJOR
> 
>     Once you start running the eigenestimates it always runs them, this is because the routine begins with
> 
>   if (cheb->kspest) {
> 
>    but once cheb->kspest is set it is never unset. This means, for example, that every time PCMG runs the smoother that uses Chebyshev it runs the eigenestimator (which uses GMRES) (even when it is suppose to be just smoothing since the eigenestimates were already made in the setup stage). This is totally wrong.
> 
> Yikes, does this code (a few lines down) address this?
> 
> if (amatid != cheb->amatid || pmatid != cheb->pmatid || amatstate != cheb->amatstate || pmatstate != cheb->pmatstate) {
> 
> Maybe you could run with CG as the outer solver and check that the number of GMRES solve calls (maybe with GMRESOrtho/max_it) is equal to the number of SNES iterations * (number of levels - 1).
>  
> Sure enough, if I run, for example, src/snes/examples/tutorials/ex19.c with -pc_type gamg I see in the debugger that GMRES is being called by KSPSolve_Chebyshev as it smooths. For example,
> 
> 0  MatSOR (mat=0x28689f0, b=0x29ee310, omega=1, flag=28, shift=0, its=1, lits=1, x=0x29f4070)
>     at /sandbox/bsmith/petsc/src/mat/interface/matrix.c:3913
> #1  0x00007f59d2e353b9 in PCApply_SOR (pc=0x29aa770, x=0x29ee310, y=0x29f4070)
>     at /sandbox/bsmith/petsc/src/ksp/pc/impls/sor/sor.c:31
> #2  0x00007f59d2fa6a7b in PCApply (pc=0x29aa770, x=0x29ee310, y=0x29f4070)
>     at /sandbox/bsmith/petsc/src/ksp/pc/interface/precon.c:462
> #3  0x00007f59d2faa6a7 in PCApplyBAorAB (pc=0x29aa770, side=PC_LEFT, x=0x29f11c0, y=0x29f4070, work=0x29ee310)
>     at /sandbox/bsmith/petsc/src/ksp/pc/interface/precon.c:691
> #4  0x00007f59d3084d46 in KSP_PCApplyBAorAB (ksp=0x29c4d30, x=0x29f11c0, y=0x29f4070, w=0x29ee310)
>     at /sandbox/bsmith/petsc/include/petsc/private/kspimpl.h:309
> #5  0x00007f59d3086874 in KSPGMRESCycle (itcount=0x7ffd3d60143c, ksp=0x29c4d30)
>     at /sandbox/bsmith/petsc/src/ksp/ksp/impls/gmres/gmres.c:152
> #6  0x00007f59d3087352 in KSPSolve_GMRES (ksp=0x29c4d30) at /sandbox/bsmith/petsc/src/ksp/ksp/impls/gmres/gmres.c:234
> #7  0x00007f59d30fae94 in KSPSolve (ksp=0x29c4d30, b=0x29dc900, x=0x29d9a30)
>     at /sandbox/bsmith/petsc/src/ksp/ksp/interface/itfunc.c:780
> #8  0x00007f59d306a1e1 in KSPSolve_Chebyshev (ksp=0x29a9550) at /sandbox/bsmith/petsc/src/ksp/ksp/impls/cheby/cheby.c:367
> #9  0x00007f59d30fae94 in KSPSolve (ksp=0x29a9550, b=0x28653d0, x=0x2906a70)
>     at /sandbox/bsmith/petsc/src/ksp/ksp/interface/itfunc.c:780
> #10 0x00007f59d2f59042 in PCMGMCycle_Private (pc=0x2832fd0, mglevelsin=0x2944b88, reason=0x0)
>     at /sandbox/bsmith/petsc/src/ksp/pc/impls/mg/mg.c:20
> #11 0x00007f59d2f5e350 in PCApply_MG (pc=0x2832fd0, b=0x28653d0, x=0x2906a70)
>     at /sandbox/bsmith/petsc/src/ksp/pc/impls/mg/mg.c:377
> #12 0x00007f59d2fa6a7b in PCApply (pc=0x2832fd0, x=0x28653d0, y=0x2906a70)
>     at /sandbox/bsmith/petsc/src/ksp/pc/interface/precon.c:462
> #13 0x00007f59d31242d7 in KSP_PCApply (ksp=0x27df750, x=0x28653d0, y=0x2906a70)
>     at /sandbox/bsmith/petsc/include/petsc/private/kspimpl.h:281
> #14 0x00007f59d31251ba in KSPInitialResidual (ksp=0x27df750, vsoln=0x28610d0, vt1=0x28ff7b0, vt2=0x2903450, 
>     vres=0x2906a70, vb=0x28653d0) at /sandbox/bsmith/petsc/src/ksp/ksp/interface/itres.c:67
> #15 0x00007f59d30872ef in KSPSolve_GMRES (ksp=0x27df750) at /sandbox/bsmith/petsc/src/ksp/ksp/impls/gmres/gmres.c:233
> #16 0x00007f59d30fae94 in KSPSolve (ksp=0x27df750, b=0x28653d0, x=0x28610d0)
>     at /sandbox/bsmith/petsc/src/ksp/ksp/interface/itfunc.c:780
> #17 0x00007f59d3291d32 in SNESSolve_NEWTONLS (snes=0x26f2550) at /sandbox/bsmith/petsc/src/snes/impls/ls/ls.c:224
> #18 0x00007f59d320f7da in SNESSolve (snes=0x26f2550, b=0x0, x=0x285d440)
> 
> 
> Can this just be the first time it is called, so it is doing the setup?
>  
>   Not only is the code wrong but it is also a huge inefficiency in the code running all these unneeded GMRES.
> 
> Just to be clear, this is inefficiency, but I don't see why it is (mathematically) wrong.
> 
> 
>   Problem 2) Less catastrophic 
> 
>   When cheb->kspest is set the "regular" Chebyshev is also run (after the eigenvalues are estimated).
> 
> I don't see that. I see:
> 
> static PetscErrorCode KSPSolve_Chebyshev(KSP ksp)
> .....
>   if (cheb->kspest) {
>     .....
>     if (amatid != cheb->amatid || pmatid != cheb->pmatid || amatstate != cheb->amatstate || pmatstate != cheb->pmatstate) {
>        ... eig est
>     }
>   }
>  .. cheby ..
> 
>  
> I am pretty sure this is wrong because there is no reason to run the Chebyshev when estimating the eigenvalues? The eigenvalues are stored to be used later right?
> 
>    So what needs to be fixed? Well somehow cheb->kspest has to be turned off once the eigenvalues are computed, but turned back on each time the matrix changes. And (easier) when cheb->kspest is used then the actual Chebyshev iterations need to be turned off.
> 
>    I hope I am wrong and the code is correct but I'm pretty sure the code is not what it should be.
> 
>    Barry
> 
> 
> 
>