Dmitry : I pushed a bugfix of mumps interface to 3.3. When merging it to petsc-dev, I get
merging src/ksp/pc/impls/fieldsplit/fieldsplit.c failed! Hong > > On Mon, Aug 20, 2012 at 8:36 AM, Jed Brown <jedbrown at mcs.anl.gov> wrote: > >> On Mon, Aug 20, 2012 at 4:02 AM, Dmitry Karpeev <karpeev at >> mcs.anl.gov>wrote: >> >>> >>> >>> On Sun, Aug 19, 2012 at 1:17 PM, Jed Brown <jedbrown at mcs.anl.gov> wrote: >>> >>>> On Sun, Aug 19, 2012 at 12:48 PM, Dmitry Karpeev <karpeev at >>>> mcs.anl.gov>wrote: >>>> >>>>> The main reason for this being a patch in 3.3 is that recursive >>>>> FieldSplit is broken there (which is why I couldn't enable it in >>>>> libMesh/Moose correctly). >>>> >>>> >>>> The current interface doesn't explicitly support all the ways to get >>>> information into the split, but several people have worked around the >>>> limitations to get the necessary information in there. Having you and Matt >>>> changing things to rely on mutually incompatible side-effects does not >>>> help. >>>> >>> I don't think the present fix is about the API or working through side >>> effects (nullspace stuff excluded and partially backed out here: >>> http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/1723d4624521) >>> >> >> What do you call this external DM futzing? >> > It's no different than holding onto the ISs set in > PCFieldSplitSetDefaults() or directly via PCFieldSplitSetIS() until they > can be used in PCSetUp_FieldSplit(). I don't see how this can be avoided > here or in the future, unless we do something to fundamentally change the > set up process. > >> >> >>> The current interface assumes that the A00 solver and the Schur inner >>> solver are to be set up identically >>> >> >> If they are being set up identically, they should literally be the same >> object. This is often the biggest setup cost in the whole problem (e.g. >> AMG). >> > I agree, but that's available only in petsc-dev, not in petsc-3.3. The > duplicate setup has been occurring all along, now it is actually consistent > with the DM being forwarded to the inner solver. > >> >> >>> (something we relaxed in petsc-dev). In order to use fieldsplit on >>> these solvers the DM for the corresponding split must be forwarded to both >>> of these A00 solvers. It was being set only on the "outer" A00 solver. >>> Partly this is an artifact of our "split" setup process: some of it occurs >>> in PCFieldSplitSetDefaults(), some later in PCSetUp_FieldSplit(). Some >>> objects obtained in PCFieldSplitSetDefaults() need to be cached until >>> PCSetUp_FieldSplit(), and the splits' DMs are among those. I now put in >>> proper reference counting for them here: >>> http://petsc.cs.iit.edu/petsc/releases/petsc-3.3/rev/188af9799779 >>> >>> >>>> Maybe we can reach some agreement on the proper way to do things. >>>> >>> That's fine with me. The reason I wanted to fix this particular problem >>> now is that some dependent packages (e.g., Moose) only rely on release >>> versions of petsc and would not be able to use this functionality properly >>> for quite some time. >>> >> >> Why do packages do this? Because petsc-dev is sometimes unstable. >> >> Now what happens when code that is pushed to the release changes behavior >> and uses uninitialized memory? The release becomes unstable. >> >> Even worse, our nightly testing is targeted at petsc-dev because the >> expectation is that only trivial and well-tested bug-fix patches are pushed >> to release. If we are changing that model by introducing significant >> changes, we MUST have nightly tests for the release, we have to actually >> look at the results, and I would be strongly in favor of switching from >> 3.3p3 numbering to 3.3.1. >> >> In all cases, these subminor or patch releases MUST be binary and >> source-level backward compatible. >> >> >>> Let's fix that "down the line". Setting a DM should never force it to be >>>> used or cause an error due to unsupported operation in a case where not >>>> having a DM is also acceptable. >>>> >>> Currently the inner and outer A00 solvers are essentially identified >>> (duplicated), so they should be set up identically, including the DM. >>> Especially when that DM defines a recursive split. This is in part >>> because there is no way to configure those two differently programmatically >>> or from the command line. So if the outer A00 is using >>> -fieldsplit_0_pc_type_fieldsplit, the inner A00 will as well. >>> However, the DM on the inner A00 will be rather different (or absent) >>> and produce splits incompatible with the rest of the options (e.g., a >>> single default split, while -fieldsplit_0_pc_fieldsplit_type schur). >>> >>> Incidentally, this raises this question down the road: in petsc-dev the >>> inner and outer A00 can be configure separately using different prefixes, >>> but how should the inner A00 DM be configured? >>> >> >> The KSPs should only be different if the solver is different. Having them >> separate, but using the same DM is one reason I was not wild about having >> physics in the DM, but I don't see a good way to plumb in the extra >> information. In any case, we need to come up with a solution before pushing >> code. >> >> >>> Okay, I'll back out that part of the second patch, since A11's >>>>> nullspace is assumed to be meant for S. I'm not sure what's incorrect >>>>> about the comment, though: if a vector is in the A11's kernel, but not in >>>>> S's, I call it a "false positive". This terminology may be wanting, but >>>>> what's incorrect about it? >>>>> >>>> >>>> "force an inconsistent rhs" >>>> >>> The KSP solve will fail only if the search space X is contracted to the >>> point where the rhs b is not in the range AX, hence, >>> inconsistent. >>> >> >> Okay, I just would have said that it changes the operator. Also, the >> Krylov methods are not necessarily correct if the matrix does not have the >> stated null space (since it's expected that the matrix has the null space >> and only the preconditioner needs to be filtered). >> >> >>> I backed out this hunk, however. >>> >>>> >>>> S may well be nonsingular. >>>> >>>> >>>>> >>>>> >>>>>> I think that's confusing and we need a specific API for it, but if >>>>>> people are using it that way, we shouldn't change it in 3.3. >>>>>> >>>>>> I'd prefer to backout the first patch, split it into separate >>>>>> pieces, and review each before pushing. I think it's a potentially big >>>>>> behavior change for 3.3. >>>>>> >>>>> Which pieces of the first patch do you think are too big for 3.3? The >>>>> splits' and Schur KSPs have to be set up (maybe not all in every >>>>> situation) >>>>> in order for recursive splitting to work. Should we declare that >>>>> capability unavailable for 3.3? I can eliminate the unneeded KSPSetUp() >>>>> calls. >>>>> >>>> >>>> 1. I hate those hacky dm reference-non-references. >>>> >>>> 2. I don't want unused KSPs to be set up. (We should eventually fix the >>>> model so they don't exist...) >>>> >>>> 3. Two of the three IncrementTabLevels that you introduced are not >>>> necessary because the KSP's tab level was incremented before getting the >>>> PC. >>>> >>> Where? >>> >> >> 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); >> + } >> >> >> 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); >> + } >> >> >> This is the pattern used everywhere else in PETSc so that sub-pcs always >> have the correct tab level. >> > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120821/385a7b46/attachment.html>
