[petsc-dev] FieldSplit fixes in 3.3

Jed Brown jedbrown at mcs.anl.gov
Wed Aug 22 06:40:04 CDT 2012


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/20120822/60216b69/attachment.html>


More information about the petsc-dev mailing list