[petsc-dev] Problematic Merge of FieldSplit
Matthew Knepley
knepley at gmail.com
Tue Jul 17 10:13:54 CDT 2012
I just pushed the resolution for this thread. I still need to tweak it to
support SIMPLE since you need
different solvers for the parts of the Schur factorization, but I will do
that soon.
Matt
On Tue, Jul 10, 2012 at 9:03 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
>
> On Jul 10, 2012, at 3:41 AM, Dmitry Karpeev wrote:
>
> > Evidently, I am confused: the current code (Matt's
> http://petsc.cs.iit.edu/petsc/petscdev/annotate/0d4ccb990bb8/src/ksp/pc/impls/fieldsplit/fieldsplit.c#l592
> )
> > as well as what I had (
> http://petsc.cs.iit.edu/petsc/petsc-dev/annotate/a36eb42b26ee/src/ksp/pc/impls/fieldsplit/fieldsplit.c#l555
> )
> > both set the inner KSP(A00) prefix to that of the outer KSP(A00). This
> is because schur's inheriting the prefix from the 0 split is
> > equivalent to setting it with PetscSNPrintf(schurprefix, sizeof
> schurprefix, "%sfieldsplit_%s_", ((PetscObject)pc)->prefix ?
> ((PetscObject)pc)->prefix : "", jac->head->splitname);CHKERRQ(ierr);
> > That works for me. Matt, I thought that didn't work for SIMPLE?
>
> 1) there should be a possibility of a different prefix so that the
> two solve approximations can be different.
>
> 2) the current code seems stupid, since both solvers are the same
> (same options) but it creates 2 different KSPs and needs to set them both
> up even though they are generally the same.
>
> I think we need to change MatCreateSchurComplement() so that it takes an
> optional KSP (and hence does not need to create its own) or change the
> PCSetUp_FieldSplit() to not create a KSP(A00) and use the one from
> MatSchurComplementGetKSP() when they should be the same. Then provide a
> PCFieldSplitSchurSetUseDifferentA00KSP() option to trigger having two
> different KSPs, each with its own prefix.
>
>
> Barry
>
>
>
> >
> > Dmitry.
> >
> > On Mon, Jul 9, 2012 at 1:56 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
> >
> > On Jul 9, 2012, at 1:17 AM, Dmitry Karpeev wrote:
> >
> > >
> > >
> > > On Sun, Jul 8, 2012 at 7:46 PM, Barry Smith <bsmith at mcs.anl.gov>
> wrote:
> > >
> > > On Jul 8, 2012, at 3:36 PM, Dmitry Karpeev wrote:
> > >
> > > > As currently designed, there are always two KSPs for A00 - one is
> created as part of MatSchurComplement and the other one sits in the FS
> split.
> > >
> > > Really? Isn't that often non-efficient? For any expensive to build
> and memory intensive preconditioner (like ILU(k), LU, AMR) it is crazy to
> create them both if they are the same.
> > > Actually, until recently these two KSPs had different options
> prefixes: the outer had -fieldsplit_0 and the inner -fieldsplit_1,
> > > so if they were set up from the command line, they could (and did for
> me) end up being different, so this reuse of the KSP would be a new thing.
> >
> >
> > Huuuh? -fieldplit_1 is most definitely NOT the prefix for the KSP
> inside the application of the Schur complement. It is for the KSP that is
> used to solve the Schur complement system.
> >
> > We need to start all over again at page 1 since one of us doesn't
> know what he is talking about.
> >
> > With block Jacobi and block Gauss-Seidel (called multiplicative in
> fieldsplit) there is a KSP for first block and a KSP for the second block;
> their prefixes are -fieldsplit_0 and fieldsplit_1.
> >
> > With the Schur complement preconditioner there are potentially three
> solves; the 0,0 block, the 0,0 block INSIDE the application of the Schur
> complement and the one associated with the Schur complement system.
> >
> > I see someone has fucked with my perfect code.
> >
> > ierr = KSPGetOptionsPrefix(jac->head->next->ksp, &Dprefix);
> CHKERRQ(ierr);
> > ierr = KSPSetOptionsPrefix(jac->kspschur, Dprefix);
> CHKERRQ(ierr);
> > /* really want setfromoptions called in
> PCSetFromOptions_FieldSplit(), but it is not ready yet */
> > /* need to call this every time, since the jac->kspschur is
> freshly created, otherwise its options never get set */
> > ierr = KSPSetFromOptions(jac->kspschur);CHKERRQ(ierr);
> >
> > Not sure what this is suppose to be doing and what is in
> jac->head->next->ksp ? If it has a level_0 that is strange.
> >
> > BTW:
> >
> > static PetscErrorCode PCApply_FieldSplit_Schur(PC pc,Vec x,Vec y)
> > {
> >
> > has
> >
> > ierr = MatSchurComplementGetKSP(jac->schur,&ksp);CHKERRQ(ierr);
> >
> > which means currently the KSP used for the 0,0 block is ALWAYS exactly
> the same as the one used inside the Schur complement?
> >
> > Or has Matt changed this?
> >
> >
> > Barry
> >
> >
> >
> >
> > > Reusing the factors from ILU(k) or LU would be a good thing, since
> the same solve is done as many as 3 times sometimes. The same would be
> true for an ASM-type preconditioner with (I)LU on the blocks -- even more
> so, since submatrices are getting pulled out.
> > >
> > >
> > > > Are you proposing that the option mechanism we have been discussing
> trigger the creation of the extra KSP,
> > >
> > > Yes (maybe not just the option mechanism, but shouldn't there be
> one that is shared by default and only two if they are different?
> > > Here's what I think we could do: if there is a -fieldsplit_0_schur_
> prefix in the database (this requires a new PetscOptionsHasNamePrefix()),
> then set this prefix on the inner KSP(A00) inside S and prefix
> -fieldsplit_0_ on the outer KSP(A00); otherwise pull out the inner KSP(A00)
> from S and reuse it as the outer KSP(A00) with prefix -fieldsplit_0_.
> > >
> > >
> > > Another scenario I can imagine (I'm not advocating this, though) is to
> have both prefixes -fieldsplit_0_ and -fieldsplit_0_schur refer
> > > to the inner and outer KSPs, if they are shared (by default), and to
> separate KSPs, if the sharing is turned off with a separate option.
> > > I'm not entirely sure how to implement this (it would require an
> aliasing mechanism in the PetscOptions database), nor whether this is
> actually reasonable.
> > >
> > > Dmitry.
> > >
> > > Barry
> > >
> > >
> > > > or do you have a different set of options in mind?
> > >
> > >
> > > >
> > > > Dmitry.
> > > >
> > > >
> > > >
> > > > On Sunday, July 8, 2012, Barry Smith <bsmith at mcs.anl.gov> wrote:
> > > > >
> > > > > On Jul 8, 2012, at 2:03 AM, Dmitry Karpeev wrote:
> > > > >
> > > > >>
> > > > >>
> > > > >> On Sat, Jul 7, 2012 at 4:01 PM, Barry Smith <bsmith at mcs.anl.gov>
> wrote:
> > > > >>
> > > > >> On Jul 7, 2012, at 3:23 AM, Dmitry Karpeev wrote:
> > > > >>
> > > > >> >
> > > > >> >
> > > > >> > On Fri, Jul 6, 2012 at 9:56 PM, Barry Smith <bsmith at mcs.anl.gov>
> wrote:
> > > > >> >
> > > > >> > On Jul 6, 2012, at 9:35 PM, Matthew Knepley wrote:
> > > > >> >
> > > > >> > > On Fri, Jul 6, 2012 at 8:18 PM, Barry Smith <
> bsmith at mcs.anl.gov> wrote:
> > > > >> > >
> > > > >> > > I don't understand this thread but I see nothing wrong
> with options like
> > > > >> > >
> > > > >> > > -fieldsplit_0_fieldsplit_0_fieldsplit_0....
> > > > >> > >
> > > > >> > > when using three nested levels of fieldsplit in the same
> way that three levels of block Jacobi (or ASM) gives -sub_sub_sub....
> > > > >> > >
> > > > >> > > The recursive nature of the prefixes should be completely
> natural and not require any special code ....
> > > > >> > >
> > > > >> > > Providing a prefix in the command line for other options
> seems terrible to me. Using the word inner also seems terrible; when you
> have _sub_sub that is clearly inner
> > > > >> > >
> > > > >> > > Could someone explain to me what prefixes are being
> generated that are not the normal recursive process and why?
> > > > >> > >
> > > > >> > > Sure, that is how it works in general. The point here is the
> distinction between A^{-1} in the (0,0) block and
> > > > >> > > A^{-1} embedded in S, which I will call A^{-1}_S. Right now
> we have
> > > > >> > >
> > > > >> > > a) A^{-1}_S and S have the same prefix
> > > > >> > >
> > > > >> > > which Dmitry does not want (perfectly reasonable).
> > > > >> > Before we had
> > > > >> > >
> > > > >> > > b) A^{-1} and A^{-1}_S had the same prefix
> > > > >> > >
> > > > >> > > which I do not want since it makes things like SIMPLE hard. I
> wanted
> > > > >> > >
> > > > >> > > c) A^{-1}_S has prefix <prefix of S>_sub
> > > > >> > >
> > > > >> > > but Dmitry said this was a hassle for normal setups and
> suggested that
> > > > >> > > we have some option that allows A^{-1} and A^{-1}_S to have
> different
> > > > >> > > prefixes.
> > > > >> > >
> > > > >> >
> > > > >> > Thanks, this makes things much clearer.
> > > > >> >
> > > > >> > 1) I don't like your prefix _sub (what the heck does _sub mean
> in this case) but I agree with you that having a different prefix there is
> good
> > > > >> >
> > > > >> > 2) I don't like Dmitry's solution. It introduces an entirely
> new paradigm that we don't have anywhere else in PETSc.
> > > > >> >
> > > > >> > My thoughts ----------
> > > > >> >
> > > > >> > For PCMG we have prefixes for the levels mg_levels_%d_
> if the user uses -mg_levels_ksp_type it applied to ALL the levels but if
> the user does -mg_levels_3_ksp_type it is applied only to the 3rd level.
> This is done by having the special treatment of _%d_ integers n the prefix
> that the options database can handle. It would be nice if we could use
> this same basic paradigm to handle this new case that supports both what
> Dmitry and you want but not in a hacky ugly special case way. For example
> (not so good) just use
> > > > >> >
> > > > >> > -fieldsplit_0_ksp_type sets the same for both
> > > > >> > -fieldsplit_0_0_ksp_type for the 0,0 block
> -fieldsplit_0_1_ksp_type for the solve inside the application of S.
> > > > >> >
> > > > >> > this is not good because it uses 0 and 1 for the two solves
> (and 0 and 1 have no particular meaning here) but the advantage is that it
> reuses current paradigms.
> > > > >> >
> > > > >> > Going further we could have
> > > > >> >
> > > > >> > -fieldsplit_0_ksp_type sets the same for both
> > > > >> > -fieldsplit_0_outter_ksp_type for the 0,0 block and
> -fieldsplit_0_inner_ksp_type for the one inside the S
> > > > >> >
> > > > >> > to implement this we would need to add support for %s in
> options prefixes. Maybe _<%s>_ so the options processing accepts a match
> with the string inside the <> or if that is not in the options database it
> accepts an option without the entire _<%s>_ This would require some small
> additions to PetscOptionsFind_private() like the
> > > > >> >
> > > > >> > if (!*flg) {
> > > > >> > PetscInt j,cnt = 0,locs[16],loce[16];
> > > > >> > size_t n;
> > > > >> > ierr = PetscStrlen(tmp,&n);CHKERRQ(ierr);
> > > > >> > /* determine the location and number of all _%d_ in the key
> */
> > > > >> > for (i=0; i< (PetscInt)n; i++) {
> > > > >> > if (tmp[i] == '_') {
> > > > >> > for (j=i+1; j< (PetscInt)n; j++) {
> > > > >> > if (tmp[j] >= '0' && tmp[j] <= '9') continue;
> > > > >> > if (tmp[j] == '_' && j > i+1) { /* found a number */
> > > > >> > locs[cnt] = i+1;
> > > > >> > loce[cnt++] = j+1;
> > > > >> > }
> > > > >> > break;
> > > > >> > }
> > > > >> > }
> > > > >> > }
> > > > >> >
> > > > >> >
> > > > >> > What does everyone think?
> > > > >> > I'm fine with this, except for the small detail that "outer"
> and "inner" infixes might be obscure to the user.
> > > > >> > I would advocate -fieldsplit_0_ksp_type by itself setting up
> both the inner and outer A^{-1}, and -fieldsplit_0_schur_ksp_type
> > > > >> > overriding the inner solver settings -- I think "Schur" is more
> descriptive then "inner" and "outer".
> > > > >>
> > > > >> So you want -fieldsplit_0_ksp_type to apply to both or to
> the 0,0 block and -fieldsplit_0_schur_ksp_type to refer to just the one
> inside the Schur but no special one just for the 0,0 block, that will
> always be determined by -fieldsplit_0_ksp_type ?
> > > > >>
> > > > >> How do you suggest we implement this in a clean way?
> > > > >>
> > > > >> Right, the subtlety is that we want to apply the _schur prefix
> to the inner object only if it exists in the options database.
> > > > >> I think this can be solved fairly simply by providing
> PetscOptionsHasNamePrefix(const char pre[], const char name[], const char
> partial[], PetscBool *match) that returns true when partial is a prefix in
> pre##name. Maybe there is a better name for this routine to avoid
> confusion of "Prefix" with pre.
> > > > >>
> > > > >> Is that clean enough? Any other ideas?
> > > > >>
> > > > > Are there always TWO ksps (one for the 0,0 solve block and one
> for the solve inside the Schur complement? There really shouldn't always be
> two since usually they are the same, right? So really we need an option to
> indicate one wants different solvers in the the two locations and when the
> one in Schur is created it gets the special prefix. Much like up and down
> smoothing in PCMG is usually the same KSP but need not be.
> > > > >
> > > > > Barry
> > > > >
> > > > >> Note another issue: the inner and outer A00 KSP share the DM
> object. That is probably okay, though,
> > > > >> because the DM is supposed to describe the (sub)problem, which is
> the same for both solvers.
> > > > >>
> > > > >> Dmitry.
> > > > >>
> > > > >>
> > > > >> Barry
> > > > >>
> > > > >> >
> > > > >> > Dmitry.
> > > > >> >
> > > > >> >
> > > > >> > Barry
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > > Matt
> > > > >> > >
> > > > >> > >
> > > > >> > > Barry
> > > > >> > >
> > > > >> > > On Jul 6, 2012, at 8:39 AM, Matthew Knepley wrote:
> > > > >> > >
> > > > >> > > > On Fri, Jul 6, 2012 at 7:28 AM, Dmitry Karpeev <
> karpeev at mcs.anl.gov> wrote:
> > > > >> > > >
> > > > >> > > > On Fri, Jul 6, 2012 at 8:17 AM, Matthew Knepley <
> knepley at gmail.com> wrote:
> > > > >> > > > On Fri, Jul 6, 2012 at 5:06 AM, Dmitry Karpeev <
> karpeev at mcs.anl.gov> wrote:
> > > > >> > > > Here's the line in question (also see the immediately
> preceding code):
> > > > >> > > >
> http://petsc.cs.iit.edu/petsc/petsc-dev/rev/0d4ccb990bb8#l1.127
> > > > >> > > >
> > > > >> > > > As long as we are fixing this, I would rather not repeat
> the prefix, since we will likely want to
> > > > >> > > > configure this differently than the block 0 solve. Is any
> thing wrong with
> > > > >> > > >
> > > > >> > > > schurprefix+"_sub"
> > > > >> > > > If the inner and outer KSP prefixes are different, it will
> force one to repeat all of the configuration options for the inner and
> outer A00 solvers, even when it is desirable to keep them identical.
> > > > >> > > > This becomes tedious, if the A00 solvers configuration is
> involved (e.g., a nested fieldsplit with separate options for the splits
> etc.).
> > > > >> > > > I would advocate making the inner solver use the same
> prefix as the outer solver by default, and allowing the user to specify
> > > > >> > > > a separate prefix for the inner solver, if it is to be
> configured differently. For example:
> > > > >> > > > -fieldsplit_0_schur_prefix fieldsplit_0_inner
> -fieldsplit_0_ksp_type gmres -fieldsplit_0_inner_ksp_type preonly etc.
> > > > >> > > >
> > > > >> > > > As long as there is a way to do it.
> > > > >> > > >
> > > > >> > > > Matt
> > > > >> > > >
> > > > >> > > > Dmitry.
> > > > >> > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > Matt
> > > > >> > > >
> > > > >> > > > Dmitry.
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > ---------- Forwarded message ----------
> > > > >> > > > From: Dmitry Karpeev <karpeev at mcs.anl.gov>
> > > > >> > > > Date: Fri, Jul 6, 2012 at 6:04 AM
> > > > >> > > > Subject: Re: [petsc-dev] Problematic Merge of FieldSplit
> > > > >> > > > To: For users of the development version of PETSc <
> petsc-dev at mcs.anl.gov>
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > I have the following problem with the prefix choice for the
> MatSchurComplement KSP introduced in this changeset (
> http://petsc.cs.iit.edu/petsc/petsc-dev/rev/0d4ccb990bb8).
> > > > >> > > > I'm talking about the "inner" KSP for A00, effecting
> inv(A00) in the definition S = A11 - A10 inv(A00) A01.
> > > > >> > > > We also have the "outer" inv(A00) KSP, which gets prefix
> "0". I recently set the "inner" inv(A00) KSP
> > > > >> > > > prefix to "0", simply by inheriting it from the "outer"
> solver. Now, it is completely reasonable
> > > > >> > > > to expect the inner and outer A00 KSPs to have different
> prefixes so that they can be configured differently.
> > > > >> > > > In fact, there was a recent petsc-users request related to
> this (http://lists.mcs.anl.gov/pipermail/petsc-users/2012-June/014005.html
> ).
> > > > >> > > > However, currently the inner A00 KSP inherits the prefix
> from the A11 KSP corresponding to the "1" field. With this prefix choice
> > > > >> > > > I end up configuring inv(A00) and inv(S) identically, which
> isn't what I want at all.
> > > > >> > > > I'm not sure what the right approach is, but the current
> one doesn't work for me.
> > > > >> > > >
> > > > >> > > > Note also that if A00 is treated with a recursive split,
> there may be numerous options for the A00 KSP.
> > > > >> > > > Do we want to repeat them for the inner and outer KSPs, if
> we want to configure them identically?
> > > > >> > > > It's automatic, if the two A00 KSPs share a prefix. Again,
> this takes away some flexibility, so maybe it's not the best solution,
> > > > >> > > > but I think retaining a simple option for using identical
> configurations is also highly desirable.
> > > > >> > > >
> > > > >> > > > Any ideas on how to handle this?
> > > > >> > > > Dmitry.
> > > > >> > > >
> > > > >> > > > On Tue, Jun 26, 2012 at 6:13 AM, Matthew Knepley <
> knepley at gmail.com> wrote:
> > > > >> > > > It turns out that 'hg rollback' during an 'hg rebase' does
> not do what I thought it did. I think
> > > > >> > > > everything is cleaned up with this push, but if you made FS
> changes in the past month, please
> > > > >> > > > check that it is doing what you want with prefixes, etc.
> > > > >> > > >
> > > > >> > > > Now, nested fieldsplits from the command line work, ala
> > > > >> > > >
> > > > >> > > > -ksp_type fgmres
> > > > >> > > > -pc_type fieldsplit -pc_fieldsplit_type additive
> > > > >> > > > -pc_fieldsplit_0_fields 0,1
> > > > >> > > > -fieldsplit_0_pc_type fieldsplit
> > > > >> > > > -fieldsplit_0_pc_fieldsplit_type schur
> -fieldsplit_0_pc_fieldsplitschur_factorization_type full
> > > > >> > > > -fieldsplit_0_fieldsplit_velocity_ksp_type preonly
> > > > >> > > > -fieldsplit_0_fieldsplit_velocity_pc_type lu
> > > > >> > > > -fieldsplit_0_fieldsplit_pressure_ksp_rtol 1e-10
> > > > >> > > > -fieldsplit_0_fieldsplit_pressure_pc_type jacobi
> > > > >> > > > -pc_fieldsplit_1_fields 2
> > > > >> > > > -fieldsplit_temperature_ksp_type preonly
> > > > >> > > > -fieldsplit_temperature_pc_type lu
> > > > >> > > >
> > > > >> > > > A split with only one field gets the field name, and
> otherwise a split number.
> > > > >> > > >
> > > > >> > > > Matt
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > What most experimenters take for granted before they begin
> their experiments is infinitely more interesting than any results to which
> their experiments lead.
> > > > >> > > > -- Norbert Wiener
> > > > >> > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > What most experimenters take for granted before they begin
> their experiments is infinitely more interesting than any results to which
> their experiments lead.
> > > > >> > > > -- Norbert Wiener
> > > > >> > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > What most experimenters take for granted before they begin
> their experiments is infinitely more interesting than any results to which
> their experiments lead.
> > > > >> > > > -- Norbert Wiener
> > > > >> > >
> > > > >> > >
> > > > >> > >
> > > > >> > >
> > > > >> > > --
> > > > >> > > What most experimenters take for granted before they begin
> their experiments is infinitely more interesting than any results to which
> their experiments lead.
> > > > >> > > -- Norbert Wiener
> > > > >> >
> > > > >> >
> > > > >>
> > > > >>
> > > > >
> > > > >
> > >
> > >
> >
> >
>
>
--
What most experimenters take for granted before they begin their
experiments is infinitely more interesting than any results to which their
experiments lead.
-- Norbert Wiener
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120717/3d6d2c14/attachment.html>
More information about the petsc-dev
mailing list