[petsc-dev] Why do we use void* instead of PetscObject in PETSc?

[Hapla Vaclav]

Something related are functions like
https://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/SNES/SNESSetConvergenceTest.html
https://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/KSP/KSPMonitorSet.html
which even have 3 parameters - the "evaluation" function, the context, and destroying function.

I have never liked this design. I think they are pretending to be "simple" for users instead of "bothering" them with having to implement a type of SNESConvergenceTest/KSPMonitor (imaginary classes at the moment). But at the end one spends much more time by finding a bug such as passing ctx instead of &ctx or something like that.

And the API is uglier, error-prone, harder to remember, etc. It splits e.g. the notion of "convergence test" into 3 complicated arguments. Compare

SNESSetConvergenceTest(SNES snes,PetscErrorCode (*SNESConvergenceTestFunction)(SNES,PetscInt,PetscReal,PetscReal,PetscReal,SNESConvergedReason*,void*),void *cctx,PetscErrorCode (*destroy)(void*))

vs

SNESSetConvergenceTest(SNES snes, SNESConvergenceTest test)

I think if E in PETSc means extensible, we should encourage users to create their own types or classes. If that is documented and there are some examples, people will just copy-paste and alter them as before but will have less problems and nicer code eventually. And they would be better prepared to implement e.g. a whole new PC.

Thanks
Vaclav


> On 2 Mar 2019, at 17:19, Dave May via petsc-dev <petsc-dev at mcs.anl.gov> wrote:
> 
> I think there would be a lot of merit (in the long run) if user contexts, such as given to KSPSetComputeOperators(), SNESSetJacobian() etc were changed to be of type PetscObject rather than void*.
> 
> Some obvious benefits would be:
> [1] Type checking!
> [2] User contexts could be readily shared using PetscObjects reference counter (PetscObjectReferenece()).
> [3] User contexts would have a communicator.
> [4] User contexts could have textual names associated with them (PetscObjectSetName()).
> [5] Internal to PETSc (e.g. within KSPComputeOperators_SNES) the header of the context could be checked to ensure the type of the PetscObject is valid (SNES in this example).
> 
> If this change was adopted, the user could readily change their code to use a PetscContainer. Such a change avoids the user having to recreate their own object model / class to deal with simple issues associated with sharing a context, or having to manage communicators within their own contexts.
> 
> I see the point that the context is "the users problem". Many PETSc examples simply use a pointer to typedef'd struct as the context. Users tend to copy examples, and really, a simple struct with a bunch of parameters is not a good object model to be followed. Would it be so horrible to require users to put "their problem" (e.g. context) inside a PetscObject?
> 
> Besides the huge volume of work required to change internal PETSc code and more over, the code of all users, are there genuinely compelling reasons to keep using void* everywhere? I cannot think of one, but I would like to hear others thoughts on this. (Maybe there are concerns with Fortran compatibility?). 
> 
> 
> Thanks
> Dave
> 
> 
>