On Wed, Aug 22, 2012 at 6:40 AM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
> On Wed, Aug 22, 2012 at 1:16 AM, Dmitry Karpeev <karpeev at mcs.anl.gov>wrote: > >> Only jac->head->ksp is being set from options in case of Schur. >> > > But only with your patch, not before. And jac->head->ksp is not used for > PCApply_FieldSplit_Schur. > > >> All inlink->ksp is set from options otherwise. >> > > Yeah, that's the non-Schur case. > > >> >>> I also don't like part of >>> http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/3c0d43fb5911 >>> >>> + /* >>> + Set from options only the A00 split. The other split's solver >>> won't be used with Schur. >>> + Should it be destroyed? Should KSPCreate() be moved here from >>> PCFieldSplitSetIS() and invoked >>> + only when necessary? >>> + */ >>> + ierr = KSPSetFromOptions(jac->head->ksp);CHKERRQ(ierr); >>> + >>> /* need to handle case when one is resetting up the preconditioner >>> */ >>> if (jac->schur) { >>> >>> 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(). >>> >> jac->head->ksp is used for the A00 solve with SCHUR_FULL. >> > > 1. It's not acceptable to call KSPSetFromOptions() in every Newton > iteration, but your code will do just that. > > 2. Show me the part of this code that uses jac->head->ksp? > Incidentally, regarding the use of jac->head->ksp: True, there is no use of jac->head->ksp -- the Schur complements "inner" solver is being used, but that's a conceptual mistake, as far as I can tell. It doesn't matter in petsc-3.3 where the inner and outer A00 solvers are identical under all circumstances. Not so in petsc-dev -- we recently went through all that trouble of introducing ways of setting up the inner and outer solvers separately -- and now the outer solver (jac->head->ksp) isn't even being used. I imagine that's just simple oversight, but once it's corrected, jac->head->ksp will have to be set up even in the Schur case. Dmitry. > > static PetscErrorCode PCApply_FieldSplit_Schur(PC pc,Vec x,Vec y) > { > PC_FieldSplit *jac = (PC_FieldSplit*)pc->data; > PetscErrorCode ierr; > KSP ksp; > PC_FieldSplitLink ilinkA = jac->head, ilinkD = ilinkA->next; > > PetscFunctionBegin; > ierr = MatSchurComplementGetKSP(jac->schur,&ksp);CHKERRQ(ierr); > > switch (jac->schurfactorization) { > [...] > case PC_FIELDSPLIT_SCHUR_FACT_FULL: > /* [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 */ > ierr = > VecScatterBegin(ilinkA->sctx,x,ilinkA->x,INSERT_VALUES,SCATTER_FORWARD);CHKERRQ(ierr); > ierr = > VecScatterEnd(ilinkA->sctx,x,ilinkA->x,INSERT_VALUES,SCATTER_FORWARD);CHKERRQ(ierr); > ierr = KSPSolve(ksp,ilinkA->x,ilinkA->y);CHKERRQ(ierr); > ierr = MatMult(jac->C,ilinkA->y,ilinkD->x);CHKERRQ(ierr); > ierr = VecScale(ilinkD->x,-1.0);CHKERRQ(ierr); > ierr = > VecScatterBegin(ilinkD->sctx,x,ilinkD->x,ADD_VALUES,SCATTER_FORWARD);CHKERRQ(ierr); > ierr = > VecScatterEnd(ilinkD->sctx,x,ilinkD->x,ADD_VALUES,SCATTER_FORWARD);CHKERRQ(ierr); > > ierr = KSPSolve(jac->kspschur,ilinkD->x,ilinkD->y);CHKERRQ(ierr); > ierr = > VecScatterBegin(ilinkD->sctx,ilinkD->y,y,INSERT_VALUES,SCATTER_REVERSE);CHKERRQ(ierr); > ierr = > VecScatterEnd(ilinkD->sctx,ilinkD->y,y,INSERT_VALUES,SCATTER_REVERSE);CHKERRQ(ierr); > > ierr = MatMult(jac->B,ilinkD->y,ilinkA->y);CHKERRQ(ierr); > ierr = VecAXPY(ilinkA->x,-1.0,ilinkA->y);CHKERRQ(ierr); > ierr = KSPSolve(ksp,ilinkA->x,ilinkA->y);CHKERRQ(ierr); > ierr = > VecScatterBegin(ilinkA->sctx,ilinkA->y,y,INSERT_VALUES,SCATTER_REVERSE);CHKERRQ(ierr); > ierr = > VecScatterEnd(ilinkA->sctx,ilinkA->y,y,INSERT_VALUES,SCATTER_REVERSE);CHKERRQ(ierr); > } > > > >> We could further classify the types of set up that are needed based on >> the type of Schur used. >> >> >>> >>> Now, checking to see what has happened in this flurry of patches and >>> partial reversions: >>> >>> $ hg diff -r dbc8d9af8577 src/ksp/pc/impls/ # revision is before all >>> of your changes >>> >>> I would like to get rid of these superfluous hunks that we just talked >>> about >>> >>> ierr = >>> KSPCreate(((PetscObject)pc)->comm,&jac->kspschur);CHKERRQ(ierr); >>> ierr = >>> PetscLogObjectParent((PetscObject)pc,(PetscObject)jac->kspschur);CHKERRQ(ierr); >>> ierr = >>> PetscObjectIncrementTabLevel((PetscObject)jac->kspschur,(PetscObject)pc,1);CHKERRQ(ierr); >>> + { >>> + PC pcschur; >>> + ierr = KSPGetPC(jac->kspschur, &pcschur); >>> CHKERRQ(ierr); >>> + ierr = >>> PetscObjectIncrementTabLevel((PetscObject)pcschur,(PetscObject)pc,1);CHKERRQ(ierr); >>> + } >>> >>> @@ -1108,6 +1152,11 @@ >>> ilink->next = PETSC_NULL; >>> ierr = >>> KSPCreate(((PetscObject)pc)->comm,&ilink->ksp);CHKERRQ(ierr); >>> ierr = >>> PetscObjectIncrementTabLevel((PetscObject)ilink->ksp,(PetscObject)pc,1);CHKERRQ(ierr); >>> + { >>> + PC ilinkpc; >>> + ierr = KSPGetPC(ilink->ksp, &ilinkpc); CHKERRQ(ierr); >>> + ierr = >>> PetscObjectIncrementTabLevel((PetscObject)ilinkpc,(PetscObject)pc,1);CHKERRQ(ierr); >>> + } >>> >> How are these superfluous? Without them -ksp_monitor formatting is wrong. >> The inner PC has to be indented, not just the KSP. >> > > 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 *one* 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? > > src/ksp/pc/impls/asm/asm.c- ierr = > KSPCreate(PETSC_COMM_SELF,&ksp);CHKERRQ(ierr); > src/ksp/pc/impls/asm/asm.c- ierr = > PetscLogObjectParent(pc,ksp);CHKERRQ(ierr); > src/ksp/pc/impls/asm/asm.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/bddc/bddc.c- ierr = > KSPCreate(PETSC_COMM_SELF,&pcbddc->ksp_D);CHKERRQ(ierr); > src/ksp/pc/impls/bddc/bddc.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)pcbddc->ksp_D,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/bddc/bddc.c- ierr = > KSPCreate(PETSC_COMM_SELF,&pcbddc->ksp_R);CHKERRQ(ierr); > src/ksp/pc/impls/bddc/bddc.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)pcbddc->ksp_R,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/bddc/bddc.c- ierr = > KSPCreate(coarse_comm,&pcbddc->coarse_ksp);CHKERRQ(ierr); > src/ksp/pc/impls/bddc/bddc.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)pcbddc->coarse_ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/bjacobi/bjacobi.c- ierr = > KSPCreate(PETSC_COMM_SELF,&ksp);CHKERRQ(ierr); > src/ksp/pc/impls/bjacobi/bjacobi.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/bjacobi/bjacobi.c- ierr = > KSPCreate(PETSC_COMM_SELF,&ksp);CHKERRQ(ierr); > src/ksp/pc/impls/bjacobi/bjacobi.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/bjacobi/bjacobi.c- ierr = > KSPCreate(subcomm,&jac->ksp[0]);CHKERRQ(ierr); > src/ksp/pc/impls/bjacobi/bjacobi.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)jac->ksp[0],(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/fieldsplit/fieldsplit.c- ierr = > KSPSetDMActive(ilink->ksp, PETSC_FALSE);CHKERRQ(ierr); > src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)dms[i],(PetscObject)ilink->ksp,0); > CHKERRQ(ierr); > -- > src/ksp/pc/impls/fieldsplit/fieldsplit.c- /* Indent this deeper to > emphasize the "inner" nature of this solver. */ > src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = > KSPIncrementTabLevel(ksp, (PetscObject) pc, 2);CHKERRQ(ierr); > -- > src/ksp/pc/impls/fieldsplit/fieldsplit.c- ierr = > PetscLogObjectParent((PetscObject)pc,(PetscObject)jac->kspschur);CHKERRQ(ierr); > src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = > KSPIncrementTabLevel(jac->kspschur,(PetscObject)pc,1); > CHKERRQ(ierr); > -- > src/ksp/pc/impls/fieldsplit/fieldsplit.c- ierr = > KSPCreate(((PetscObject)pc)->comm,&ilink->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = > KSPIncrementTabLevel(ilink->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/fieldsplit/fieldsplit.c- ierr = > KSPCreate(((PetscObject)pc)->comm,&ilink->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/fieldsplit/fieldsplit.c: ierr = > KSPIncrementTabLevel(ilink->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/galerkin/galerkin.c- ierr = > KSPCreate(((PetscObject)pc)->comm,&jac->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/galerkin/galerkin.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)jac->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/gasm/gasm.c- ierr = > PetscLogObjectParent(pc,ksp);CHKERRQ(ierr); > src/ksp/pc/impls/gasm/gasm.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/is/nn/nn.c- ierr = > KSPCreate(((PetscObject)pc)->comm,&pcnn->ksp_coarse);CHKERRQ(ierr); > src/ksp/pc/impls/is/nn/nn.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)pcnn->ksp_coarse,(PetscObject)pc,2);CHKERRQ(ierr); > -- > src/ksp/pc/impls/is/pcis.c- ierr = > KSPCreate(PETSC_COMM_SELF,&pcis->ksp_D);CHKERRQ(ierr); > src/ksp/pc/impls/is/pcis.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)pcis->ksp_D,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/is/pcis.c- ierr = > KSPCreate(PETSC_COMM_SELF,&pcis->ksp_N);CHKERRQ(ierr); > src/ksp/pc/impls/is/pcis.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)pcis->ksp_N,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/ksp/pcksp.c- ierr = > KSPCreate(((PetscObject)pc)->comm,&jac->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/ksp/pcksp.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)jac->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/lsc/lsc.c- ierr = > KSPCreate(((PetscObject)pc)->comm,&lsc->kspL);CHKERRQ(ierr); > src/ksp/pc/impls/lsc/lsc.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)lsc->kspL,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/mg/mg.c- ierr = PCSetType(ipc,PCSOR);CHKERRQ(ierr); > src/ksp/pc/impls/mg/mg.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)mglevels[i]->smoothd,(PetscObject)pc,levels-i);CHKERRQ(ierr); > -- > src/ksp/pc/impls/mg/mgfunc.c- ierr = > KSPCreate(comm,&mglevels[l]->smoothu);CHKERRQ(ierr); > src/ksp/pc/impls/mg/mgfunc.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)mglevels[l]->smoothu,(PetscObject)pc,mglevels[0]->levels-l);CHKERRQ(ierr); > -- > src/ksp/pc/impls/openmp/hpc.c- ierr = > KSPCreate(((PetscObject)pc)->comm,&red->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/openmp/hpc.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)red->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/redistribute/redistribute.c- ierr = > KSPCreate(((PetscObject)pc)->comm,&red->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/redistribute/redistribute.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)red->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/redundant/redundant.c- ierr = > KSPCreate(subcomm,&red->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/redundant/redundant.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)red->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/redundant/redundant.c- ierr = > KSPCreate(subcomm,&red->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/redundant/redundant.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)red->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > -- > src/ksp/pc/impls/wb/wb.c- ierr = > KSPCreate(PETSC_COMM_SELF,&ctx->ksp);CHKERRQ(ierr); > src/ksp/pc/impls/wb/wb.c: ierr = > PetscObjectIncrementTabLevel((PetscObject)ctx->ksp,(PetscObject)pc,1);CHKERRQ(ierr); > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120827/aa486d22/attachment.html>