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.

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 <hzh...@mcs.anl.gov>
Sent: Saturday, April 25, 2020 2:40 PM
To: Pierre Jolivet <pierre.joli...@enseeiht.fr>
Cc: Jose E. Roman <jro...@dsic.upv.es>; Stefano Zampini 
<stefano.zamp...@gmail.com>; petsc-dev <petsc-dev@mcs.anl.gov>; Smith, Barry F. 
<bsm...@mcs.anl.gov>
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 <pierre.joli...@enseeiht.fr>
Sent: Saturday, April 25, 2020 11:45 AM
To: Zhang, Hong <hzh...@mcs.anl.gov>
Cc: Jose E. Roman <jro...@dsic.upv.es>; Stefano Zampini 
<stefano.zamp...@gmail.com>; petsc-dev <petsc-dev@mcs.anl.gov>; Smith, Barry F. 
<bsm...@mcs.anl.gov>
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 
<hzh...@mcs.anl.gov<mailto:hzh...@mcs.anl.gov>> 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, 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

Reply via email to