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

[Barry Smith]

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".

  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.

   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