<br><br><div class="gmail_quote">On Wed, Aug 22, 2012 at 6:40 AM, Jed Brown <span dir="ltr"><<a href="mailto:jedbrown@mcs.anl.gov" target="_blank">jedbrown@mcs.anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div>On Wed, Aug 22, 2012 at 1:16 AM, Dmitry Karpeev <span dir="ltr"><<a href="mailto:karpeev@mcs.anl.gov" target="_blank">karpeev@mcs.anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>Only jac->head->ksp is being set from options in case of Schur. </div></blockquote><div><br></div></div><div>But only with your patch, not before. And jac->head->ksp is not used for PCApply_FieldSplit_Schur.</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div> All inlink->ksp is set from options otherwise.</div></blockquote><div><br></div></div><div>Yeah, that's the non-Schur case.</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div>
<br>I also don't like part of <a href="http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/3c0d43fb5911" target="_blank">http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/3c0d43fb5911</a><br></div><div><br><div>+ /* </div>
<div>
+ Set from options only the A00 split. The other split's solver won't be used with Schur. </div><div>+ Should it be destroyed? Should KSPCreate() be moved here from PCFieldSplitSetIS() and invoked </div>
<div>+ only when necessary? </div><div>+ */</div><div>+ ierr = KSPSetFromOptions(jac->head->ksp);CHKERRQ(ierr);</div><div>+</div><div> /* need to handle case when one is resetting up the preconditioner */</div>
<div> if (jac->schur) {</div></div><div><br></div><div>This call is being done every time through PCSetUp_FieldSplit(), which it shouldn't be. It's also being called on a KSP that is not being used for anything (e.g. look at the "if (jac->type == PC_COMPOSITE_SCHUR) {" part of PCSetUp_FieldSplit().<br>
</div></div></blockquote></div><div>jac->head->ksp is used for the A00 solve with SCHUR_FULL. </div></blockquote><div><br></div></div><div>1. It's not acceptable to call KSPSetFromOptions() in every Newton iteration, but your code will do just that.</div>
<div><br></div><div>2. Show me the part of this code that uses jac->head->ksp?</div></div></blockquote><div>Incidentally, regarding the use of jac->head->ksp: </div><div>True, there is no use of jac->head->ksp -- the Schur complements "inner" solver is being used, </div>
<div>but that's a conceptual mistake, as far as I can tell. It doesn't matter in petsc-3.3 where the inner </div><div>and outer A00 solvers are identical under all circumstances. Not so in petsc-dev -- we recently went</div>
<div>through all that trouble of introducing ways of setting up the inner and outer solvers separately -- and</div><div>now the outer solver (jac->head->ksp) isn't even being used. I imagine that's just simple oversight,</div>
<div>but once it's corrected, jac->head->ksp will have to be set up even in the Schur case. </div><div><br></div><div>Dmitry.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote">
<div><br></div><div><div>static PetscErrorCode PCApply_FieldSplit_Schur(PC pc,Vec x,Vec y)</div><div>{</div><div> PC_FieldSplit *jac = (PC_FieldSplit*)pc->data;</div>
<div> PetscErrorCode ierr;</div><div> KSP ksp;</div><div> PC_FieldSplitLink ilinkA = jac->head, ilinkD = ilinkA->next;</div><div><br></div><div> PetscFunctionBegin;</div><div><div> ierr = MatSchurComplementGetKSP(jac->schur,&ksp);CHKERRQ(ierr);</div>
<div><br></div></div><div> switch (jac->schurfactorization) {</div><div>[...]</div><div> case PC_FIELDSPLIT_SCHUR_FACT_FULL:</div><div> /* [1 0; A10 A00^{-1} 1] [A00 0; 0 S] [1 A00^{-1}A01; 0 1], an exact solve if applied exactly, needs one extra solve with A */</div>
<div> ierr = VecScatterBegin(ilinkA->sctx,x,ilinkA->x,INSERT_VALUES,SCATTER_FORWARD);CHKERRQ(ierr);</div><div> ierr = VecScatterEnd(ilinkA->sctx,x,ilinkA->x,INSERT_VALUES,SCATTER_FORWARD);CHKERRQ(ierr);</div>
<div> ierr = KSPSolve(ksp,ilinkA->x,ilinkA->y);CHKERRQ(ierr);</div><div> ierr = MatMult(jac->C,ilinkA->y,ilinkD->x);CHKERRQ(ierr);</div><div> ierr = VecScale(ilinkD->x,-1.0);CHKERRQ(ierr);</div>
<div> ierr = VecScatterBegin(ilinkD->sctx,x,ilinkD->x,ADD_VALUES,SCATTER_FORWARD);CHKERRQ(ierr);</div><div> ierr = VecScatterEnd(ilinkD->sctx,x,ilinkD->x,ADD_VALUES,SCATTER_FORWARD);CHKERRQ(ierr);</div>
<div><br></div><div> ierr = KSPSolve(jac->kspschur,ilinkD->x,ilinkD->y);CHKERRQ(ierr);</div><div> ierr = VecScatterBegin(ilinkD->sctx,ilinkD->y,y,INSERT_VALUES,SCATTER_REVERSE);CHKERRQ(ierr);</div>
<div> ierr = VecScatterEnd(ilinkD->sctx,ilinkD->y,y,INSERT_VALUES,SCATTER_REVERSE);CHKERRQ(ierr);</div><div><br></div><div> ierr = MatMult(jac->B,ilinkD->y,ilinkA->y);CHKERRQ(ierr);</div><div> ierr = VecAXPY(ilinkA->x,-1.0,ilinkA->y);CHKERRQ(ierr);</div>
<div> ierr = KSPSolve(ksp,ilinkA->x,ilinkA->y);CHKERRQ(ierr);</div><div> ierr = VecScatterBegin(ilinkA->sctx,ilinkA->y,y,INSERT_VALUES,SCATTER_REVERSE);CHKERRQ(ierr);</div><div> ierr = VecScatterEnd(ilinkA->sctx,ilinkA->y,y,INSERT_VALUES,SCATTER_REVERSE);CHKERRQ(ierr);</div>
<div> }</div></div><div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div> We could further classify the types of set up that are needed based on the type of Schur used.</div>
<div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div>
<br><br>Now, checking to see what has happened in this flurry of patches and partial reversions:<br><br>$ hg diff -r dbc8d9af8577 src/ksp/pc/impls/ # revision is before all of your changes<br><br>I would like to get rid of these superfluous hunks that we just talked about</div>
<div>
<div><br><div> ierr = KSPCreate(((PetscObject)pc)->comm,&jac->kspschur);CHKERRQ(ierr);</div><div> ierr = PetscLogObjectParent((PetscObject)pc,(PetscObject)jac->kspschur);CHKERRQ(ierr);</div><div>
ierr = PetscObjectIncrementTabLevel((PetscObject)jac->kspschur,(PetscObject)pc,1);CHKERRQ(ierr);</div><div>+ {</div><div>+ PC pcschur;</div><div>+ ierr = KSPGetPC(jac->kspschur, &pcschur); CHKERRQ(ierr);</div>
<div>+ ierr = PetscObjectIncrementTabLevel((PetscObject)pcschur,(PetscObject)pc,1);CHKERRQ(ierr);</div><div>+ }</div></div></div><div><br><div>@@ -1108,6 +1152,11 @@</div><div> ilink->next = PETSC_NULL;</div>
<div>
<div> ierr = KSPCreate(((PetscObject)pc)->comm,&ilink->ksp);CHKERRQ(ierr);</div><div> ierr = PetscObjectIncrementTabLevel((PetscObject)ilink->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>+ { </div><div>+ PC ilinkpc;</div><div>+ ierr = KSPGetPC(ilink->ksp, &ilinkpc); CHKERRQ(ierr);</div><div>+ ierr = PetscObjectIncrementTabLevel((PetscObject)ilinkpc,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>+ }</div></div></div></div></blockquote></div><div>How are these superfluous? Without them -ksp_monitor formatting is wrong. The inner PC has to be indented, not just the KSP.</div></blockquote></div></div><br><div>
Come on, we just discussed this a few messages earlier in this very thread. Tons of PCs create inner KSPs, but all of them behave correctly without KSPIncrementTabLevel() because they increment the tab level *before* the inner PC is created. Two out of three places in fieldsplit.c also follow this pattern, therefore the old code was fine. As far as I can tell, there is only <b>one</b> place in all of PETSc that requires KSPIncrementTabLevel() and it is because MatCreateSchurComplement() cannot use a KSP that has been passed in, yet it calls KSPSetOperators() which forces creation of the inner PC. Perhaps we should get rid of KSPIncrementTabLevel() so people don't get confused and conclude that they need to use it?</div>
<div><br>src/ksp/pc/impls/asm/asm.c- ierr = KSPCreate(PETSC_COMM_SELF,&ksp);CHKERRQ(ierr);</div><div><div>src/ksp/pc/impls/asm/asm.c- ierr = PetscLogObjectParent(pc,ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/asm/asm.c: ierr = PetscObjectIncrementTabLevel((PetscObject)ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/bddc/bddc.c- ierr = KSPCreate(PETSC_COMM_SELF,&pcbddc->ksp_D);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/bddc/bddc.c: ierr = PetscObjectIncrementTabLevel((PetscObject)pcbddc->ksp_D,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/bddc/bddc.c- ierr = KSPCreate(PETSC_COMM_SELF,&pcbddc->ksp_R);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/bddc/bddc.c: ierr = PetscObjectIncrementTabLevel((PetscObject)pcbddc->ksp_R,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/bddc/bddc.c- ierr = KSPCreate(coarse_comm,&pcbddc->coarse_ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/bddc/bddc.c: ierr = PetscObjectIncrementTabLevel((PetscObject)pcbddc->coarse_ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/bjacobi/bjacobi.c- ierr = KSPCreate(PETSC_COMM_SELF,&ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/bjacobi/bjacobi.c: ierr = PetscObjectIncrementTabLevel((PetscObject)ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/bjacobi/bjacobi.c- ierr = KSPCreate(PETSC_COMM_SELF,&ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/bjacobi/bjacobi.c: ierr = PetscObjectIncrementTabLevel((PetscObject)ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/bjacobi/bjacobi.c- ierr = KSPCreate(subcomm,&jac->ksp[0]);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/bjacobi/bjacobi.c: ierr = PetscObjectIncrementTabLevel((PetscObject)jac->ksp[0],(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c- ierr = KSPSetDMActive(ilink->ksp, PETSC_FALSE);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = PetscObjectIncrementTabLevel((PetscObject)dms[i],(PetscObject)ilink->ksp,0); CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c- /* Indent this deeper to emphasize the "inner" nature of this solver. */</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = KSPIncrementTabLevel(ksp, (PetscObject) pc, 2);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c- ierr = PetscLogObjectParent((PetscObject)pc,(PetscObject)jac->kspschur);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = KSPIncrementTabLevel(jac->kspschur,(PetscObject)pc,1); CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c- ierr = KSPCreate(((PetscObject)pc)->comm,&ilink->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = KSPIncrementTabLevel(ilink->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c- ierr = KSPCreate(((PetscObject)pc)->comm,&ilink->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = KSPIncrementTabLevel(ilink->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/galerkin/galerkin.c- ierr = KSPCreate(((PetscObject)pc)->comm,&jac->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/galerkin/galerkin.c: ierr = PetscObjectIncrementTabLevel((PetscObject)jac->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/gasm/gasm.c- ierr = PetscLogObjectParent(pc,ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/gasm/gasm.c: ierr = PetscObjectIncrementTabLevel((PetscObject)ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/is/nn/nn.c- ierr = KSPCreate(((PetscObject)pc)->comm,&pcnn->ksp_coarse);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/is/nn/nn.c: ierr = PetscObjectIncrementTabLevel((PetscObject)pcnn->ksp_coarse,(PetscObject)pc,2);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/is/pcis.c- ierr = KSPCreate(PETSC_COMM_SELF,&pcis->ksp_D);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/is/pcis.c: ierr = PetscObjectIncrementTabLevel((PetscObject)pcis->ksp_D,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/is/pcis.c- ierr = KSPCreate(PETSC_COMM_SELF,&pcis->ksp_N);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/is/pcis.c: ierr = PetscObjectIncrementTabLevel((PetscObject)pcis->ksp_N,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/ksp/pcksp.c- ierr = KSPCreate(((PetscObject)pc)->comm,&jac->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/ksp/pcksp.c: ierr = PetscObjectIncrementTabLevel((PetscObject)jac->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/lsc/lsc.c- ierr = KSPCreate(((PetscObject)pc)->comm,&lsc->kspL);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/lsc/lsc.c: ierr = PetscObjectIncrementTabLevel((PetscObject)lsc->kspL,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/mg/mg.c- ierr = PCSetType(ipc,PCSOR);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/mg/mg.c: ierr = PetscObjectIncrementTabLevel((PetscObject)mglevels[i]->smoothd,(PetscObject)pc,levels-i);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/mg/mgfunc.c- ierr = KSPCreate(comm,&mglevels[l]->smoothu);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/mg/mgfunc.c: ierr = PetscObjectIncrementTabLevel((PetscObject)mglevels[l]->smoothu,(PetscObject)pc,mglevels[0]->levels-l);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/openmp/hpc.c- ierr = KSPCreate(((PetscObject)pc)->comm,&red->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/openmp/hpc.c: ierr = PetscObjectIncrementTabLevel((PetscObject)red->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/redistribute/redistribute.c- ierr = KSPCreate(((PetscObject)pc)->comm,&red->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/redistribute/redistribute.c: ierr = PetscObjectIncrementTabLevel((PetscObject)red->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/redundant/redundant.c- ierr = KSPCreate(subcomm,&red->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/redundant/redundant.c: ierr = PetscObjectIncrementTabLevel((PetscObject)red->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/redundant/redundant.c- ierr = KSPCreate(subcomm,&red->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/redundant/redundant.c: ierr = PetscObjectIncrementTabLevel((PetscObject)red->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
<div>--</div><div>src/ksp/pc/impls/wb/wb.c- ierr = KSPCreate(PETSC_COMM_SELF,&ctx->ksp);CHKERRQ(ierr);</div><div>src/ksp/pc/impls/wb/wb.c: ierr = PetscObjectIncrementTabLevel((PetscObject)ctx->ksp,(PetscObject)pc,1);CHKERRQ(ierr);</div>
</div><div><br></div>
</blockquote></div><br>