[petsc-dev] How to enforce private data/methods in PETSc?

[Jed Brown]

Matthew Knepley <knepley at gmail.com> writes:

> On Sat, Aug 11, 2018 at 12:02 AM Junchao Zhang <jczhang at mcs.anl.gov> wrote:
>
>> See this example, after multiple casts, the code directly accesses
>> VecScatter_MPI_General's members, with an assumption that from->n is the
>> total number of receive processors. When I separate intranode / internode
>> communications, I have to maintain this assumption.
>>
>> This is a good example. VecScatter is a flagrant rule violator. It should
> be rewritten with a proper API
> to obtain this data.

Also note that this code (which is in Mat) needs to

  #include <petsc/private/vecimpl.h>

which should ideally never happen.  That rule has been broken sometimes
and it hasn't been a priority to create internal interfaces to fix these
historical code smells (most VecScatter code dates from the mid-1990s),
but we do try to avoid it in new code and refactor when making
significant changes to old code.

Note that in C++, you could

  #define private public
  #define protected public

before including a header, and then could access any member.

>
> And if one was going to rewrite it, my personal prejudice would be to try
> and replace the communication
> part with implementations of PetscSF. Right now SF can handle different
> data types (Scatter cannot), but
> Scatter has many more modes of communication, such as Alltoall, whereas SF
> only has p2p and window.
>
>    Matt
>
>> 385: *PetscErrorCode <http://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/Sys/PetscErrorCode.html#PetscErrorCode> MatMatMultNumeric_MPIDense(Mat <http://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/Mat/Mat.html#Mat> A,Mat <http://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/Mat/Mat.html#Mat> B,Mat <http://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/Mat/Mat.html#Mat> C)*386: {
>>
>> 389:   Mat_MPIAIJ             *aij = (Mat_MPIAIJ*) A->data;
>> 393:   VecScatter <http://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/Vec/VecScatter.html#VecScatter>             ctx   = aij->Mvctx;394:   VecScatter_MPI_General *from = (VecScatter_MPI_General*) ctx->fromdata;395:   VecScatter_MPI_General *to   = (VecScatter_MPI_General*) ctx->todata;
>> 411:   PetscMalloc4 <http://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/Sys/PetscMalloc4.html#PetscMalloc4>(B->cmap->N*from->starts[from->n],&contents->rvalues,412:                       B->cmap->N*to->starts[to->n],&contents->svalues,413:                       from->n,&contents->rwaits,414:                       to->n,&contents->swaits);
>>
>>
>> --Junchao Zhang
>>
>>
>> On Fri, Aug 10, 2018 at 10:33 PM Matthew Knepley <knepley at gmail.com>
>> wrote:
>>
>>> On Fri, Aug 10, 2018 at 11:29 PM Junchao Zhang <jczhang at mcs.anl.gov>
>>> wrote:
>>>
>>>> It seems we do not have naming conventions for private members.
>>>>
>>>
>>> I am not sure I understand. There are no public members. For private
>>> functions, we do have
>>> a naming convention, but it is newly created, so many of the existing
>>> functions break the rules.
>>>
>>>    Matt
>>>
>>>
>>>> --Junchao Zhang
>>>>
>>>>
>>>> On Fri, Aug 10, 2018 at 9:43 PM Matthew Knepley <knepley at gmail.com>
>>>> wrote:
>>>>
>>>>> On Fri, Aug 10, 2018 at 5:43 PM Junchao Zhang <jczhang at mcs.anl.gov>
>>>>> wrote:
>>>>>
>>>>>>  I met several bugs that remind me to raise this question. In PETSc,
>>>>>> object of type A can arbitrarily access object of type B's data. But
>>>>>> designer of B may later change the meaning of its data (and of course,
>>>>>> update B's interfaces, which are usually local to few files). The designer
>>>>>> may think the job is done, but actually it is not.  He/she has to grep the
>>>>>> code to know where its data members are accessed (that is relatively easy
>>>>>> to get) and what is the contract, for example, is an array assumed to be
>>>>>> sorted (that is hard to know).  With C++, one can use private to minimize
>>>>>> data exposure.
>>>>>>
>>>>>
>>>>> This just has to be coding discipline. People should not be accessing
>>>>> private members.
>>>>>
>>>>>    Matt
>>>>>
>>>>>
>>>>>> --Junchao Zhang
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> What most experimenters take for granted before they begin their
>>>>> experiments is infinitely more interesting than any results to which their
>>>>> experiments lead.
>>>>> -- Norbert Wiener
>>>>>
>>>>> https://www.cse.buffalo.edu/~knepley/ <http://www.caam.rice.edu/~mk51/>
>>>>>
>>>>
>>>
>>> --
>>> What most experimenters take for granted before they begin their
>>> experiments is infinitely more interesting than any results to which their
>>> experiments lead.
>>> -- Norbert Wiener
>>>
>>> https://www.cse.buffalo.edu/~knepley/ <http://www.caam.rice.edu/~mk51/>
>>>
>>
>
> -- 
> What most experimenters take for granted before they begin their
> experiments is infinitely more interesting than any results to which their
> experiments lead.
> -- Norbert Wiener
>
> https://www.cse.buffalo.edu/~knepley/ <http://www.caam.rice.edu/~mk51/>