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