On Mon, Aug 27, 2012 at 4:44 PM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
> I did more reverting > > http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/2de903ed8221 > > fixed bad formatting/whitespace > > http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/9bd051edbe68 > > and merged with petsc-dev (discarding functional changes from 3.3) > > http://petsc.cs.iit.edu/petsc/petsc-dev/rev/518adbf64b18 > > > 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. > Okay, I'll do that shortly. I've just tested that this will work with recursive splits (at least the ones I obtain using libMesh/Moose). Dmitry. > > > 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? >> >> 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/5c5ba5d2/attachment-0001.html>
