[petsc-dev] [petsc-maint] Feature: allow unsorted row IS in MatGetSubMatrix_SeqAIJ()

[Barry Smith]

  Toby,

   Sounds ok to me. Thanks Barry

> On Jan 20, 2015, at 6:44 PM, Tobin Isaac <tisaac at ices.utexas.edu> wrote:
> 
> On Mon, Jan 19, 2015 at 02:06:16PM -0600, Barry Smith wrote:
>> 
>>> On Jan 19, 2015, at 1:36 PM, Tobin Isaac <tisaac at ices.utexas.edu> wrote:
>>> 
>>> If being sorted is a requirement of the interface, I think the ISSorted tests belong in the interface and not in the _SeqAIJ implementation; if it's not a requirement, then only the implementations that don't support unordered IS's should raise an ERR_SUP, right?
>> 
>>   Yes. But it appears with regard to sorting we've been rather scattershot on following your logic :-).
> 
> If I enforce the latter logic (raising errors) for the other
> implementations and update the doc to reflect that, would that work?
> 
>> 
>> 
>>> 
>>> On January 19, 2015 12:57:45 PM CST, Barry Smith <bsmith at mcs.anl.gov> wrote:
>>>> 
>>>> Should actually send these things to pets-dev
>>>> 
>>>> The docs for MatGetSubMatrix() have the line
>>>> 
>>>> "The rows in isrow will be sorted into the same order as the original
>>>> matrix on each process."
>>>> 
>>>>  The docs for MatGetSubMatrices() say 
>>>> 
>>>> "Currently both row and column indices must be sorted to guarantee
>>>> correctness with all matrix types."
>>>> 
>>>> PetscErrorCode MatGetSubMatrices_MPIAIJ(Mat C,PetscInt ismax,const IS
>>>> isrow[],const IS iscol[],MatReuse scall,Mat *submat[])
>>>> {
>>>> PetscErrorCode ierr;
>>>> PetscInt       nmax,nstages_local,nstages,i,pos,max_no,nrow,ncol;
>>>> PetscBool     
>>>> rowflag,colflag,wantallmatrix=PETSC_FALSE,twantallmatrix,*allcolumns;
>>>> 
>>>> PetscFunctionBegin;
>>>> /* Currently, unsorted column indices will result in inverted column
>>>> indices in the resulting submatrices. */
>>>> /* It would make sense to error out in
>>>> MatGetSubMatrices_MPIAIJ_Local(), the most impl-specific level.
>>>> However, there are more careful users of
>>>> MatGetSubMatrices_MPIAIJ_Local() -- MatPermute_MPIAIJ() -- that
>>>> take care to order the result correctly by assembling it with
>>>> MatSetValues() (after preallocating).
>>>> */
>>>> for (i = 0; i < ismax; ++i) {
>>>>  PetscBool sorted;
>>>>  ierr = ISSorted(iscol[i], &sorted);CHKERRQ(ierr);
>>>> if (!sorted) SETERRQ1(PETSC_COMM_SELF, PETSC_ERR_SUP, "Column index set
>>>> %D not sorted", i);
>>>> }
>>>> 
>>>> In other words the sorted business seems a little wacky at the moment. 
>>>> I'm not sure what to conclude.
>>>> 
>>>> 
>>>> Barry
>>>> 
>>>>> On Jan 19, 2015, at 11:36 AM, Tobin Isaac <tisaac at ices.utexas.edu>
>>>> wrote:
>>>>> 
>>>>> 
>>>>> There doesn't seem to be any reason for MatGetSubMatrix_SeqAIJ() to
>>>>> require the row IS to be sorted, and this requirement differs from
>>>>> other implementations, e.g. _MPIAIJ.  I have a feature branch that
>>>>> removes the requirement.  Can I merge it into next for testing?
>>>>> 
>>>>> Thanks,
>>>>> Toby
>>> 
>>