[petsc-dev] FieldSplit fixes in 3.3
Dmitry Karpeev
karpeev at mcs.anl.gov
Mon Aug 27 18:08:26 CDT 2012
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>
More information about the petsc-dev
mailing list