[petsc-dev] FieldSplit fixes in 3.3
Jed Brown
jedbrown at mcs.anl.gov
Mon Aug 27 16:44:13 CDT 2012
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.
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/8548345c/attachment.html>
More information about the petsc-dev
mailing list