Hong If the product is not supported, the type of C will never be set anyway, so you cannot call MatHasOperation after MatProductSetFromOptions. The purpose of MatProductSetFromOptions is to populate the function pointers for symbolic and numeric phases. If not found, they should be set to null instead of erroring as it is now. What I propose is to have MatProductHasOperation (not MatHasOperation): this function will be identical to MatHasOperation, with the only difference that does not call PetscValidType on the input mat.
Meanwhile, I’m coding a basic MatMat (and MatTransposeMat) driver to loop over dense columns and apply MatMult. (Or MatMultTranspose) without memory movement. This will be valid for all B matrices being of type dense (and its derivations), with C of type dense too. This in principle will fix Jose and Pierre’s issues (they can correct me if I’m wrong) However, we should definitely have a way for the user to enquire if a given operation is supported or not. Thanks Stefano > On May 6, 2020, at 12:03 AM, Zhang, Hong <[email protected]> wrote: > > Stefano: > Now, we need address this bug report: enable > MatHasOperation(C,MATOP_MAT_MULT,&flg) for matrix products, e.g., C=A*B, > which is related to your issue https://gitlab.com/petsc/petsc/-/issues/608 > <https://gitlab.com/petsc/petsc/-/issues/608>. > > In petsc-3.13: > 1) MATOP_MAT_MULT, ..., MATOP_MATMAT_MULT are removed from the MATOP table > (they are still listed in petscmat.h -- an overlook, I'll remove them). > MATOP_MAT_MULT_SYMBOLIC/NUMERIC ... are still in the table. > 2) MatHasOperation(C,...) must be called for the matrix product C, not matrix > A or B (slepc needs to fix this after this reported bug is fixed). > > Like MatSetOption(), MatHasOperation() must be called AFTER MatSetType(). You > moved MatSetType() from MatProductSetFromOptions() back to > MatProductSymbolic() in your latest patch, thus user has to call > MatHasOption() after MatProductSymbolic(): > > MatProductCreate(A,B,NULL,&C); > MatProductSetType(C,...); > ... > MatProductSetFromOptions(); //if the product is not supported for the given > mat types, currently petsc crashes here, which we can replace with an error > output > > MatProductSymbloc(); -> call MatSetType() > MatHasOperation(C,MATOP_MAT_MULT,&flg) > > Question: how to call MatHasOperation(C,..) when MatProductSymbloc() is not > supported? > > My fix to this bug: > Resume MatSetType() in MatProductSetFromOptions(). Then user calls: > > MatProductCreate(A,B,NULL,&C); > MatProductSetType(C,...); > ... > MatProductSetFromOptions(C); //if the product is not supported for the given > mat types, C->ops->productsymbolic=NULL; > MatHasOperation(C,MATOP_PRODUCTSYMBOLIC,&flg); > if (flg) { > MatProductSymbolic(C); > ... > } else { > MatDestroy(&C); > ... > } > > Either you take care of this bug report, or let me know your thoughts about > how to fix this bug. > Hong > From: Zhang, Hong <[email protected]> > Sent: Saturday, April 25, 2020 2:40 PM > To: Pierre Jolivet <[email protected]> > Cc: Jose E. Roman <[email protected]>; Stefano Zampini > <[email protected]>; petsc-dev <[email protected]>; Smith, Barry > F. <[email protected]> > Subject: Re: [petsc-dev] MATOP_MAT_MULT > > Pierre, > When we do > MatProductCreate: C = A*B; //C owns A and B, thus B->refct =2 > MatProductCreateWithMats: B = A*C; //If I let B own A and C, then C->refct=2 > Then > MatDestroy(&B) and MatDestroy(&C) only reduce their refct from 2 to 1, thus > memory leak. > My solution is adding > { > matreference; /* do not add refct when using > MatProductCreateWithMat() to void recursive references */ > } Mat_Product > This flg prevents MatProductCreateWithMats() to increase reference counts, > i.e., B does not own A and C to avoid reverse ownership. I am not sure this > is a reasonable solution. Let me know if you have better solution. > See ex109.c and ex195.c for tests. > Hong > From: Pierre Jolivet <[email protected]> > Sent: Saturday, April 25, 2020 11:45 AM > To: Zhang, Hong <[email protected]> > Cc: Jose E. Roman <[email protected]>; Stefano Zampini > <[email protected]>; petsc-dev <[email protected]>; Smith, Barry > F. <[email protected]> > Subject: Re: [petsc-dev] MATOP_MAT_MULT > > Hong, > José didn’t report this, though he may have run into the same issue, I did. > I’ll try the branch and get back at you on GitLab MR. > > Thanks, > Pierre > >> On 25 Apr 2020, at 6:17 PM, Zhang, Hong <[email protected] >> <mailto:[email protected]>> wrote: >> >> Jose, >> >> >> I also now just tested some previously PETSC_VERSION_LT(3,13,0) running >> >> code with C=A*B, Dense=Nest*Dense, all previously allocated prior to a >> >> call to MatMatMult and scall = MAT_REUSE_MATRIX. >> >> Sadly, it’s now broken. It is my fault for not having a test for this in >> >> https://gitlab.com/petsc/petsc/-/merge_requests/2069 >> >> <https://gitlab.com/petsc/petsc/-/merge_requests/2069>, sorry about that. >> >> [0]PETSC ERROR: Call MatProductSymbolic() first >> >> [0]PETSC ERROR: #1 MatProductNumeric() line 730 in >> >> /ccc/work/cont003/rndm/rndm/petsc/src/mat/interface/matproduct.c >> >> [0]PETSC ERROR: #2 MatMatMult() line 9335 in >> >> /ccc/work/cont003/rndm/rndm/petsc/src/mat/interface/matrix.c >> >> >> >> Here is a reproducer (that will work OK with 3.12.4). >> >> diff --git a/src/mat/tests/ex195.c b/src/mat/tests/ex195.c >> >> index c72662bc3c..811de669c5 100644 >> >> --- a/src/mat/tests/ex195.c >> >> +++ b/src/mat/tests/ex195.c >> >> @@ -73,2 +73,3 @@ int main(int argc,char **args) >> >> ierr = >> >> MatMatMult(nest,B,MAT_REUSE_MATRIX,PETSC_DEFAULT,&C);CHKERRQ(ierr); >> >> + ierr = >> >> MatMatMult(nest,C,MAT_REUSE_MATRIX,PETSC_DEFAULT,&B);CHKERRQ(ierr); >> >> ierr = MatMatMultEqual(nest,B,C,10,&equal);CHKERRQ(ierr); >> >> >> >> $ make -f gmakefile test searchin=mat_tests-ex195 >> >> >> >> I believe this is very close to the topic at hand and issue #608, so >> >> maybe you could fix this as well in the same upcoming MR? Just let me >> >> know, I can have a crack it otherwise. >> >> This is a bug. I fixed it in the branch hzhang/fix-matproduct-reuse/maint. >> Can you test it? >> Hong
