On Fri, 27 Mar 2020 at 05:09, [email protected] <[email protected]> wrote:
> Lisandro, > >> As I said in previous emails, I'm not complaining about the API, I said >> it was a welcome adition. All I'm asking is: >> >> 1) Fix the handling of reference counting. This is an implementation >> detail, not really an API change. >> > I'm creating a branch from master and patch it with your suggestions on > reference counting. > OK, thanks! > >> 2) Add a simple new routine to the API, MatProductReset(D) [or maybe >> name it MatProductClear(D) ], that should throw away all of the D->product >> stuff to release memory related to the product algorithm implementation. >> After calling MatProductReset(D), you cannot call anymore >> MatProductNumeric(). >> > Do you mean adding a public function MatProductClear(D) to clear internal > data structure 'product' attached to D? > For example, after destroy A and B, user should call MatProductClear(D), > i.e., > MatDestroy(&A) ; > MatDestroy(&B) ; > MatProductClear(D); // throw away all of the D->product stuff, thus D > becomes a standard matrix > > Exactly. MatProductClear(D) should make D a standard matrix and discard all of the internal product stuff. > I understand that you are concerned about the dangling pointers. However, > most users compute D =A*B, then destroy A, B and D separately in random > order. Do we require user to call MatProductClear(D) after A or B is > destroyed? > No, MatProductClear() should not be required. I would code MatProductClear(D) to be a non-op if D does not have the internal product stuff. Then I would always call MatProductClear(D) in MatDestroy(&D) . Of course, all that assuming that MatProductCreate() makes D take ownership (increment refcount) of A and B (and C if given). This way it does not really matter the order in which users destroy A,B, and D. As I said before, look at PCReset() and PCDestroy(). MatProcuctClear() should work similarly as the Reset() routine. > >>> 3) For MatProduct, I consider >>> MatProductSetFromOptions() is logically equivalent to >>> MatSetFromOptions(), >>> MatProductSymbolic() is logically equivalent to MatSetUp(), >>> and >>> MatProductNumeric() assembles the matproduct D repeatedly when A or B >>> are updated. >>> >>> >> OK, so... your point it that what looks like "setup" code >> in MatProductSetFromOptions() is just some minimal stuff required to >> actually dispatch the SetFromOptions_typeA_typeB routine, right? In that >> case, you are right, no changes/refactors needed here. >> > Yes, MatProductSetFromOptions() only > dispatches SetFromOptions_typeA_typeB_producttype() which handles local > options and polymorphism. The organization of new API is described at the > top of petsc/src/mat/interface/matproduct.c. > > OK, all good with that one. Regards, -- Lisandro Dalcin ============ Research Scientist Extreme Computing Research Center (ECRC) King Abdullah University of Science and Technology (KAUST) http://ecrc.kaust.edu.sa/
