I did more reverting<br><br><div><a href="http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/2de903ed8221">http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/2de903ed8221</a><div><br>fixed bad formatting/whitespace<div>
<br></div><div><a href="http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/9bd051edbe68">http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/9bd051edbe68</a></div><div><br></div><div>and merged with petsc-dev (discarding functional changes from 3.3)<br>
<br><a href="http://petsc.cs.iit.edu/petsc/petsc-dev/rev/518adbf64b18">http://petsc.cs.iit.edu/petsc/petsc-dev/rev/518adbf64b18</a><br><br><br>Dmitry, if you want the extra DM forwarding in petsc-dev, it should be done by KSPGetDM(). This can and should forward the DMShell. If that causes something else to misbehave, we will fix the misbehaving thing.<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 class="im">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 class="im">
<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 class="im">
<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><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 class="im"><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 class="im"><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></div></div></div>