The 'saved' context is in submatrices, not original matrix. I made following
fix:
if (C->submat_singleis) { /* flag is set in PCSetUp_ASM() to skip
MPIU_Allreduce() */
ierr =
MatGetSubMatrices_MPIAIJ_SingleIS(C,ismax,isrow,iscol,scall,submat);CHKERRQ(ierr);
+ C->submat_singleis = PETSC_FALSE; /* resume its default value in case C
will be used for non-singlis */
PetscFunctionReturn(0);
}
...
+ } else { /* MAT_REUSE_MATRIX */
subc = (Mat_SeqAIJ*)((*submat)[0]->data);
smat = subc->submatis1;
if (!smat) {
/* smat is not generated by
MatGetSubMatrix_MPIAIJ_All(...,MAT_INITIAL_MATRIX,...) */
wantallmatrix = PETSC_TRUE;
+ } else if (smat->singleis) {
+ ierr =
MatGetSubMatrices_MPIAIJ_SingleIS(C,ismax,isrow,iscol,scall,submat);CHKERRQ(ierr);
+ PetscFunctionReturn(0);
} else {
...
ie., after first call of MatGetSubMatrices_MPIAIJ_SingleIS, this flag is set in
saved context of submatrices
and is reset as 'false' to the original matrix C.
Do you agree with this fix? Is so, I'll push it in a branch for nightly test.
Hong
________________________________________
From: Barry Smith [[email protected]]
Sent: Friday, January 20, 2017 8:19 PM
To: Matthew Knepley
Cc: Zhang, Hong; PETSc
Subject: Re: [petsc-dev] Bug introduced in MatGetSubmatrices()
Ok, so the problem comes from setting the flag in in the original matrix that
other people have access to and can be used for other purposes by anyone,.
So it would be better to instead put the flag into the generated sub matrix
which no one else has access to. This should be do-able and is the right way to
do it anyways.
Hong, can you try putting all the "saved" context into the generated
submatrices instead of the original matrix? If that is not possible then the
context saved with the original matrix should have a reference to the sub
matrix and this context should only be reused if the sub matrix passed in
matches the sub matrix referenced in the context (this is a less desirable
solution).
Barry
> On Jan 20, 2017, at 8:06 PM, Matthew Knepley <[email protected]> wrote:
>
> On Fri, Jan 20, 2017 at 7:39 PM, Barry Smith <[email protected]> wrote:
>
> > On Jan 20, 2017, at 7:01 PM, Matthew Knepley <[email protected]> wrote:
> >
> > On Fri, Jan 20, 2017 at 3:08 PM, Barry Smith <[email protected]> wrote:
> >
> > > On Jan 20, 2017, at 3:01 PM, Zhang, Hong <[email protected]> wrote:
> > >
> > > We set C->submat_singleis = false as default.
> > > User turns it on when he knows a single IS will be used for
> > > MatGetSubMatrices(), e.g., when asm is used.
> > > User should turn the flag off after it is being used.
> >
> > Right, this is why I am confused why this is messing up Mat since
> > presumably he is not setting it?
> >
> > ASM is turning this flag on, and it is corrupting my UNRELATED call to
> > MatGetSubMatrices().
>
> Yes, but you need to tell us exactly how. The flag is set into
> C->submat_singleis which is associated with the C from ASM, so why is it
> affecting your UNRELATED call to MatGetSubMatrices(). It is not a global
> variable so shouldn't.
>
> It sets the flag on the primary matrix. I will go step by step
>
> 1) I have a linear system with matrix A
>
> 2) To form a preconditioner for FieldSplit, I extract some diagonal blocks
> from A using MatGetSubMatrices()
>
> 3) I also use ASM on the another block of FS
>
> 4) The second Newton iterate, I try to call MatGetSubMatrices(), but ASM
> has set a flag on the matrix that causes this to fail.
>
> Explain or send a code that reproduces the problem.
>
> You can easily reproduce this by installing PyLith (a widely used package)
> and running an automatic test. However, since this
> is so simple, I am not sure this is necessary.
>
> Matt
>
> Barry
>
> >
> > Matt
> >
> > >
> > > This flag is for optimization -- avoid MPI_Allreduce, which is expensive
> > > for np>10k.
> > > MatGetSubMatrices() works without this flag turns on.
> > > I just merged the improved MatGetSubMatrices_MPIAIJ() which only calls
> > > MPI_Allreduce once
> > > for repeated use. Previously, it calls MPI_Allreduce twice for each call
> > > of MatGetSubMatrices_MPIAIJ().
> > >
> > > Hong
> > >
> > > ________________________________________
> > > From: Barry Smith [[email protected]]
> > > Sent: Friday, January 20, 2017 1:11 PM
> > > To: Matthew Knepley
> > > Cc: Zhang, Hong; PETSc
> > > Subject: Re: [petsc-dev] Bug introduced in MatGetSubmatrices()
> > >
> > >> On Jan 20, 2017, at 12:45 PM, Matthew Knepley <[email protected]> wrote:
> > >>
> > >> On Fri, Jan 20, 2017 at 11:55 AM, Barry Smith <[email protected]> wrote:
> > >>
> > >>> On Jan 20, 2017, at 11:49 AM, Matthew Knepley <[email protected]> wrote:
> > >>>
> > >>> On Fri, Jan 20, 2017 at 10:49 AM, Hong <[email protected]> wrote:
> > >>> Matt,
> > >>> By default, the flag C->submat_singleis = false.
> > >>> In PCSetUp_ASM(), we set it as 'true' to use
> > >>> MatGetSubMatrices_MPIAIJ_SingleIS().
> > >>>
> > >>> Can you check the value of this flag in your case?
> > >>>
> > >>> The problem is the following:
> > >>>
> > >>> 1) We use MatGetSubMatrices() to extract small matrices in order to
> > >>> form a preconditioner
> > >>>
> > >>> 2) We do this at each Newton iteration
> > >>>
> > >>> 3) We use ASM as a preconditioner for the eventual Newton solve
> > >>>
> > >>> 4) The second time we call MatGetSubMatrices(), it has this flag set,
> > >>> even though we are using multiple ISes
> > >>>
> > >>> Solution: ALSO check that the user is in fact passing a single IS.
> > >>>
> > >> This requires a communication. I am confused, is the number of IS
> > >> changing each time? If not why is the flag set?
> > >>
> > >> ASM sets this flag because it knows that IT is going to call
> > >> MatGetSubMatrices() later, but it unsafe if any user calls
> > >> MatGetSubMatrices() as well. I think overall its a fragile design and
> > >> should be scrapped.
> > >
> > > You mean the user calls the SAME MatGetSubMatrices with the same
> > > matrices, right? Not a completely different unrelated MatGetSubMatrices()
> > > which should not be affect by the previous unrelated call.
> > >
> > >
> > > Barry
> > >
> > >>
> > >> Matt
> > >>
> > >>
> > >> Barry
> > >>
> > >>> Matt
> > >>>
> > >>> Hong
> > >>>
> > >>> It comes from here:
> > >>>
> > >>>
> > >>> https://bitbucket.org/petsc/petsc/commits/c10200c1442b553b7ad65c70101560db4fa22e78
> > >>>
> > >>> If we ask for more than 1 matrix, it dispatches to
> > >>>
> > >>> MatGetSubMatrices_MPIAIJ_SingleIS()
> > >>>
> > >>> but then fails here
> > >>>
> > >>>
> > >>> https://bitbucket.org/petsc/petsc/annotate/2e559809f9aee9c95ee79eb0939630cfe5502c8d/src/mat/impls/aij/mpi/mpiov.c?at=master&fileviewer=file-view-default#mpiov.c-1306
> > >>>
> > >>> because ismax > 1. I think the ismax check needs to move up to here
> > >>>
> > >>>
> > >>> https://bitbucket.org/petsc/petsc/annotate/2e559809f9aee9c95ee79eb0939630cfe5502c8d/src/mat/impls/aij/mpi/mpiov.c?at=master&fileviewer=file-view-default#mpiov.c-2012
> > >>>
> > >>> but I don't know for sure. Please fix this since it is breaking PyLith.
> > >>>
> > >>> 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
