[petsc-dev] I do not think this is the right solution

Matthew Knepley knepley at gmail.com
Sun Mar 18 18:45:28 CDT 2012 

On Sun, Mar 18, 2012 at 6:38 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:

>
> On Mar 18, 2012, at 6:24 PM, Matthew Knepley wrote:
>
> > On Sun, Mar 18, 2012 at 5:47 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
> >
> > > Let me clarify. I did not find your completely undocumented "feature"
> that has "always been that way". I found the places where
> > > values would have to be communicated, which is what the documentation
> says the assembly does.
> >
> >    Here is the documentation of the two flags. In no way shape or form
> does it say anything about these flags in relation to communication!
> >
> >  PetscBool              assembled;        /* is the matrix assembled? */
> >  PetscBool              was_assembled;    /* new values inserted into
> assembled mat */
> >
> >   Here is the documentation for MatAssemblyBegin/End  Again this does
> not say that this function was in anyway only connected to communication
> >
> >   MatAssemblyBegin - Begins assembling the matrix.  This routine should
> >   be called after completing all calls to MatSetValues().
> >
> >  You made an assumption about the flags that was not correct, there is
> more to sticking values into the matrices then the communication and these
> two flags have always handled everything not just an indication of
> communication needed.
> >
> > > Here is what I do not understand. Isn't it obvious when an insertion
> changes nonzero structure? Shouldn't we fix this obvious functionality
> > > hole rather than complicating the interface?
> >
> >   PETSc was written with the separated setvalues phase and
>  usematrix/accessvalues phases, this is clearly documented and permeates
> the use of Mat and has always been fundamental to the design and usage of
> PETS. Suddenly YOU decide to change this model, on your own, and do a
> half-assed job of it in the process.  Did you even run the complete test
> suite and make sure your change didn't break anything?
> >
> >   We could change the model (though I think changing it is a bad idea)
> but being such a fundamental change it should have been discussed and if
> changed the users manual, the tutorials, and examples would all need major
> revision to describe the new model (which is actually more complicated than
> the old model since the meaning of MatSetValues() would be different
> depending on the current state of the matrix.)
> >
> >   So let's discuss changing the model. In my model the setvalues phase
> the usematrix/accessvalues phase must always be separated with
> MatAssemblyBegin/End. In Matt's model one can apparently call
> MatSetValues() sometimes after an assembly and then not call
> MatAssemblyBegin/End() so
> >
> > In the alternate model, you call MatAssemblyBegin/End() when the matrix
> is marked as unassembled. Its easy to check this flag, so the user
> > will not be confused. Barry is correct that the flag reset is not
> collective, so that query would have to be.
> >
> > long as one does not do communication? Or is it change the nonzero
> pattern? What if some processes do not change the nonzero pattern but
> others do, well then the user has to know this case and have all processes
> call MatAssemblyBegin/End() everywhere if any process changes the pattern
> (for example if one process changes its off-diagonal pattern no
> communication is needed to move value, but ALL processes must cooperate in
> setting up the new scatter for the multiply, this is normally handled by
> the required MatAssemblyBegin/End() process? Your model is fragile and
> confusing, yes it could be made to work but I think is not as good a model.
> >
> > I do not think "fragile" is an accurate characterization because failure
> is immediate when trying to use a function that requires assembly, and it
> > also obvious what the fix is, to assemble the matrix.
> >
> > You might mean that the existing model is more intuitive. I disagree
> since it is currently impossible to do something simple like
> > chop out small values from a matrix.
>
>    Using MatSetValues() to set those locations to zero is fine, but so
> what, really the reason to chop out small values would be to also remove
> the nonzero locations from the matrix which your model cannot do anyway and
> would likely require methods for each matrix format directly. Yes, the
> current model is not good for allowing people to manipulate matrix entries
> directly but I don't really care, that is not what PETSc is for.


No, I don't want the nonzeros removed, I want the values set to zero,
exactly like I said. This is for robustly checking
that a matrix created in parallel is the same as one made on a different
number of processors. Something I do for ex62,
but we obviously do not give a shit about doing.

    Matt


> > The proposal is to augment the current model with MatGetRowWrite(). This
> would mean
> > not only adding to the interface, but writing all the code to put values
> back into the matrix if the row is a copy. I think it is more
> > intuitive to allow the user to SetValues() if the nonzero pattern is
> unchanged and no values need to be communicated.
>
>    I do not trust the users to use this alternative use of MatSetValues()
> correctly.  I won't even add the MatGetRowWrite() method unless a useful
> use case appears, certainly zeroing small values is not such a case.
>
>   Barry
>
> >
> > However, I do not care enough if no one else thinks its easier to have
> one smarter method for value insertion, than two more
> > restrictive methods.
> >
> >     Matt
> >
> >   Barry
> >
> >
> >
> > On Mar 18, 2012, at 3:07 PM, Matthew Knepley wrote:
> >
> > > On Sun, Mar 18, 2012 at 2:49 PM, Barry Smith <bsmith at mcs.anl.gov>
> wrote:
> > >
> > > On Mar 18, 2012, at 12:09 PM, Matthew Knepley wrote:
> > >
> > > > On Sun, Mar 18, 2012 at 11:52 AM, Barry Smith <bsmith at mcs.anl.gov>
> wrote:
> > > >
> > > > On Mar 18, 2012, at 11:39 AM, Matthew Knepley wrote:
> > > >
> > > > > https://bitbucket.org/petsc/petsc-dev/changeset/7ae47bfbd6e9
> > > > >
> > > > > Why are we using the assembled flag to determine whether garray
> and colmap
> > > > > should be created? That seems like an abuse of the flag. Shouldn't
> we check
> > > > > for these when we need them?
> > > >
> > > >   This has always been the use of that flag since day one. That is
> the reason for the existence of that flag. Now you want to change the
> meaning of the flag.
> > > >
> > > >   Since your MatGetRow() MatSetValues() paradigm only works properly
> if no new nonzeros are introduced (otherwise people will get assembled
> errors after they've put in some values) what is wrong with my proposed
> solution of MatGetRowWrite() which is the correct interface for changing
> some values in the row but not the nonzero structure.
> > > >
> > > > Your own metric. It complicates the interface. I have to know that I
> only want to change existing nonzeros, when it is
> > > > perfectly easy for SetValues() to detect this AND I already went and
> found all the places where this happens.
> > >
> > >   Found all the places where what happens?  You introduced a bug, for
> example mat/examples/tests/ex37.c with multiple processes crashed after you
> "found all the places where this happens".  I wasted hours tracking down
> the problem from you missing "one of the places where this happens". And
> how come you didn't change all the other MatSetValuesXXX() that also use
> the same assembled flags? I don't think you came close to "found all the
> places where this happened".
> > >
> > > Let me clarify. I did not find your completely undocumented "feature"
> that has "always been that way". I found the places where
> > > values would have to be communicated, which is what the documentation
> says the assembly does.
> > >
> > >  PETSc Mat has ALWAYS had a simple well-defined policy of a phase
> where you set values in the matrix and a separate phase where you can
> access values; it has served us very well over the years. Now you suddenly
> what to change this policy "sometimes".
> > >
> > >   I also think you misunderstand my proposal that SAFELY gives you
> everything you want in a clean way that does not confuse the setvalues
> phase and the accessvalues phase.
> > >
> > >    MatGetRowWrite(mat,PetscInt row,PetscInt *nz, const PetscInt
> **indices,PetscScalar **values)
> > >    /*   here I have code that changes some of the entries in values;
> note I cannot change the indices since they are const */
> > >     MatRestoreRowWrite(mat,PetscInt row,PetscInt *nz, const PetscInt
> **indices,PetscScalar **values)  the routine puts the changed values back
> into the underly Mat. no chance of changing nonzero structure.
> > >
> > >    With your model
> > >
> > >    MatGetRow(mat,PetscInt row,PetscInt *nz, const PetscInt
> **indices,const PetscScalar **values)
> > >     MatRestoreRow(mat,PetscInt row,PetscInt *nz, const PetscInt
> **indices,const PetscScalar **values)
> > >    MatSetValues(mat,row ....)
> > >
> > >     there is no mechanism to insure that the user only sets values
> that already existed in the matrix which may trigger a change in the
> nonzero structure which means the next call to MatGetRow() will fail. Plus
> it posses the problem why can you sometimes mix accessing values and
> calling MatSetValues() but sometimes not? Having one simple policy on
> separating the phases is valuable and should be kept.
> > >
> > > Here is what I do not understand. Isn't it obvious when an insertion
> changes nonzero structure? Shouldn't we fix this obvious functionality
> > > hole rather than complicating the interface?
> > >
> > > I just changed the code I had to Assemble() all over the place.
> > >
> > >    Matt
> > >
> > >   Barry
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >    Barry
> > >
> > > >
> > > >    Matt
> > > >
> > > >
> > > >   Barry
> > > >
> > > > >
> > > > >    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/20120318/3edff8d2/attachment.html>

More information about the petsc-dev mailing list