Hong, It works nicely. thanks. If you are fine, I can push it to maint and add a test case in ex4.c
Stefano 2018-01-15 22:10 GMT+03:00 Hong <[email protected]>: > Stefano: > This patch > diff --git a/src/mat/impls/aij/seq/aij.c b/src/mat/impls/aij/seq/aij.c > index df16825..5c40de6 100644 > --- a/src/mat/impls/aij/seq/aij.c > +++ b/src/mat/impls/aij/seq/aij.c > @@ -2542,11 +2542,14 @@ PetscErrorCode MatDestroySubMatrices_SeqAIJ(PetscInt > n,Mat *mat[]) > c = (Mat_SeqAIJ*)C->data; > submatj = c->submatis1; > if (submatj) { > - ierr = submatj->destroy(C);CHKERRQ(ierr); > - ierr = MatDestroySubMatrix_Private(submatj);CHKERRQ(ierr); > - ierr = PetscLayoutDestroy(&C->rmap);CHKERRQ(ierr); > - ierr = PetscLayoutDestroy(&C->cmap);CHKERRQ(ierr); > - ierr = PetscHeaderDestroy(&C);CHKERRQ(ierr); > + if (--((PetscObject)C)->refct > 0) { > + } else { > + ierr = submatj->destroy(C);CHKERRQ(ierr); > + ierr = MatDestroySubMatrix_Private(submatj);CHKERRQ(ierr); > + ierr = PetscLayoutDestroy(&C->rmap);CHKERRQ(ierr); > + ierr = PetscLayoutDestroy(&C->cmap);CHKERRQ(ierr); > + ierr = PetscHeaderDestroy(&C);CHKERRQ(ierr); > + } > } else { > > should fix the problem. Let me know your thought. > Hong > > On Sun, Jan 14, 2018 at 9:09 PM, Hong <[email protected]> wrote: > >> Stefano: >> Sequential version works because it does not use common data structure. >> The old version of MatCreateSubMatrices() does not reuse internal data >> structures, thus inefficient. >> During its optimization, the internal data structures are introduced. >> >> I'll check it and see if I can provide these type of usage. >> >> Hong >> >> >>> On Jan 14, 2018, at 7:16 AM, Hong <[email protected]> wrote: >>> >>> Stefano, >>> MatCreateSubMatrices() must be destroyed by MatDestroySubMatrices() >>> because all submatrices in the array share common internal data >>> structure for reuse. >>> >>> + ierr = PetscObjectReference((PetscObject)submat);CHKERRQ(ierr); >>> >>> Only submat = submatrices[0] adds reference count. >>> >>> ierr = MatDestroySubMatrices(1,&submatrices);CHKERRQ(ierr); >>> Now, the internal common data structure is destroyed. >>> >>> + ierr = MatDestroy(&submat);CHKERRQ(ierr); >>> I guess submat becomes an orphan, it cannot be destroyed using previous >>> ops->destroy routine. >>> >>> Shall we allow user to pick a single matrix out of submatrices array, >>> and use/destroy it as a regular single matrix? >>> >>> >>> I accept the fact that the array of submatrices obtained by >>> MatCreateSubMatrices shall be destroyed by MatDestroySubMatrices. >>> However, not being able to take reference on one of the submats is not >>> properly PETSc philosophy :-), as the Mats returned do not behave like >>> proper Mat objects >>> How difficult is to allow this? and what is the common data structure >>> the submats share? >>> >>> Hong >>> >>> On Sat, Jan 13, 2018 at 1:09 PM, Stefano Zampini < >>> [email protected]> wrote: >>> >>>> Cc'ing petsc-dev >>>> ---------- Messaggio inoltrato ---------- >>>> Da: "Stefano Zampini" <[email protected]> >>>> Data: 12 Gen 2018 9:32 PM >>>> Oggetto: Re: MatDestroySubMatrices_SeqAIJ and reference counting >>>> A: "Hong" <[email protected]> >>>> Cc: >>>> >>>> >>>> Sorry, I do not understand your question. Where comes matrix B? >>>> >>>> >>>> It’s not important where matrix B comes from. The error is reproducible >>>> by just increasing the reference count of any of the submats. See below the >>>> patch for ex4.c >>>> >>>> Can you give me an example of this error? >>>> >>>> >>>> here you have >>>> >>>> *diff --git a/src/mat/examples/tests/ex4.c >>>> b/src/mat/examples/tests/ex4.c* >>>> *index 0555a54d9d..1d1961e4fa 100644* >>>> *--- a/src/mat/examples/tests/ex4.c* >>>> *+++ b/src/mat/examples/tests/ex4.c* >>>> @@ -68,7 +68,9 @@ int main(int argc,char **argv) >>>> ierr = MatView(submat,sviewer);CHKERRQ(ierr); >>>> ierr = PetscViewerRestoreSubViewer(PETSC_VIEWER_STDOUT_WORLD,PETSC_ >>>> COMM_SELF,&sviewer);CHKERRQ(ierr); >>>> ierr = PetscViewerFlush(PETSC_VIEWER_STDOUT_WORLD);CHKERRQ(ierr); >>>> + ierr = PetscObjectReference((PetscObject)submat);CHKERRQ(ierr); >>>> ierr = MatDestroySubMatrices(1,&submatrices);CHKERRQ(ierr); >>>> + ierr = MatDestroy(&submat);CHKERRQ(ierr); >>>> >>>> /* Form submatrix with rows 2-4 and all columns */ >>>> ierr = ISDestroy(&icol);CHKERRQ(ierr); >>>> >>>> Hong >>>> >>>> Hong, >>>>> >>>>> can you explain what’s the rationale behind calling explicitly the >>>>> Layout and header destroy for the submit case in the loop here >>>>> https://bitbucket.org/petsc/petsc/src/ac3af1b492556bac9 >>>>> 856a6aee1c73992bd0b1779/src/mat/impls/aij/seq/aij.c?at=maste >>>>> r&fileviewer=file-view-default#aij.c-2531 >>>>> >>>>> A code like this fails since you don’t take into account reference >>>>> counting on the submatrices. >>>>> >>>>> MatCreateSubMatrices(A,….&submats) >>>>> PetscObjectCompose(B,”_XXXX”,submats[0); >>>>> ….. >>>>> MatDestroySubMatrices(..,submats); >>>>> MatDestroy(B); //Error, corrupt argument when trying to destroy >>>>> composed objects >>>>> >>>>> Here is a representative valgrind stack trace >>>>> >>>>> ==90133== Invalid read of size 4 >>>>> ==90133== at 0x100368F63: PetscCheckPointer >>>>> (/Users/szampini/software/petsc/src/sys/error/checkptr.c:108) >>>>> ==90133== by 0x1001ACA9F: PetscObjectDereference >>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:564) >>>>> ==90133== by 0x10019AC07: PetscObjectListDestroy >>>>> (/Users/szampini/software/petsc/src/sys/objects/olist.c:154) >>>>> ==90133== by 0x1001A8176: PetscHeaderDestroy_Private >>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:115) >>>>> ==90133== by 0x100626CAD: MatDestroy (/Users/szampini/software/pets >>>>> c/src/mat/interface/matrix.c:1237) >>>>> ==90133== Address 0x106c4c320 is 1,632 bytes inside a block of size >>>>> 4,420 free'd >>>>> ==90133== at 0x10014C9F3: free (in /usr/local/Cellar/valgrind/3.1 >>>>> 3.0/lib/valgrind/vgpreload_memcheck-amd64-darwin.so) >>>>> ==90133== by 0x100229B7A: PetscFreeAlign >>>>> (/Users/szampini/software/petsc/src/sys/memory/mal.c:88) >>>>> ==90133== by 0x10022D771: PetscTrFreeDefault >>>>> (/Users/szampini/software/petsc/src/sys/memory/mtr.c:309) >>>>> ==90133== by 0x100A7034D: MatDestroySubMatrices_SeqAIJ >>>>> (/Users/szampini/software/petsc/src/mat/impls/aij/seq/aij.c:2549) >>>>> ==90133== by 0x1006260BD: MatDestroySubMatrices >>>>> (/Users/szampini/software/petsc/src/mat/interface/matrix.c:6957) >>>>> ==90133== by 0x1006D82EC: _DestroyContainer >>>>> ==90133== by 0x1001AFA31: PetscContainerDestroy >>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:899) >>>>> ==90133== by 0x1001ACBE5: PetscObjectDereference >>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:566) >>>>> ==90133== by 0x10019AC07: PetscObjectListDestroy >>>>> (/Users/szampini/software/petsc/src/sys/objects/olist.c:154) >>>>> ==90133== by 0x1001A8176: PetscHeaderDestroy_Private >>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:115) >>>>> ==90133== by 0x100626CAD: MatDestroy (/Users/szampini/software/pets >>>>> c/src/mat/interface/matrix.c:1237) >>>>> ==90133== Block was alloc'd at >>>>> ==90133== at 0x10014C616: malloc (in /usr/local/Cellar/valgrind/3.1 >>>>> 3.0/lib/valgrind/vgpreload_memcheck-amd64-darwin.so) >>>>> ==90133== by 0x1002299AC: PetscMallocAlign >>>>> (/Users/szampini/software/petsc/src/sys/memory/mal.c:48) >>>>> ==90133== by 0x10022CA73: PetscTrMallocDefault >>>>> (/Users/szampini/software/petsc/src/sys/memory/mtr.c:183) >>>>> ==90133== by 0x10022B328: PetscMallocA >>>>> (/Users/szampini/software/petsc/src/sys/memory/mal.c:396) >>>>> ==90133== by 0x100C12E05: MatCreate (/Users/szampini/software/pets >>>>> c/src/mat/utils/gcreate.c:89) >>>>> ==90133== by 0x100B839F6: MatCreateSubMatrices_MPIAIJ_Local >>>>> (/Users/szampini/software/petsc/src/mat/impls/aij/mpi/mpiov.c:2554) >>>>> ==90133== by 0x100B7DEE7: MatCreateSubMatrices_MPIAIJ >>>>> (/Users/szampini/software/petsc/src/mat/impls/aij/mpi/mpiov.c:2038) >>>>> ==90133== by 0x10066703A: MatCreateSubMatrices >>>>> (/Users/szampini/software/petsc/src/mat/interface/matrix.c:6806) >>>>> >>>>> >>>> >>>> >>>> >>> >>> >> > -- Stefano
