[petsc-dev] inconsistency for its own sake?

[Jed Brown]

On Fri, Jan 11, 2013 at 12:58 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:

>
> On Jan 11, 2013, at 12:12 PM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
>
> > Ah, Dependency versus Duplication. I blame the lack of transparency on
> the editors, which could display the definition of the typedef instead of
> having to jump to it. I like that the typedefs allow us to document the
> function in one place and that changing arguments (hopefully only while
> working out the initial implementation, but whenever it's deemed necessary)
> is more reliable. Since dynamic functions are cast to void(*)(void), the
> compiler may not be able to check that types match. Also, ability to grep
> for any place that a given callback is manipulated.
> >
> > Pros of typedef:
> > * typedef is, in principle, "safer" for developers
> > * typedef helps avoid duplication in documentation; maintains symmetry
> between Get() and Set()
> > * typedef uses simpler type notation (nice for less experienced C
> programmers)
> >
>
>     I will give more information about my plan that removes some (but not
> all) of your concerns
>
> /*M
>      SNESFunction - function used to convey the nonlinear function to be
> solved by SNES
>
>      Calling sequence of func
>
>    SNESFunction(SNES snes,Vec x,Vec f,void *ctx);
>
> snes    - the SNES context
> x       - state at which to evaluate residual
> f       - vector to put residual
> ctx     - optional user-defined function context
>
> .seealso:   SNESSetFunction()
> M*/
>
> #undef __FUNCT__
> #define __FUNCT__ "SNESSetFunction"
> /*@C
>    SNESSetFunction - Sets the function evaluation routine and function
>    vector for use by the SNES routines in solving systems of nonlinear
>    equations.
>
>    Logically Collective on SNES
>
>    Input Parameters:
> +  snes - the SNES context
> .  r - vector to store function value
> .  SNESFunction - function evaluation routine
> -  ctx - [optional] user-defined context for private data for the
>          function evaluation routine (may be PETSC_NULL)
>
>    Notes:
>    The Newton-like methods typically solve linear systems of the form
> $      f'(x) x = -f(x),
>    where f'(x) denotes the Jacobian matrix and f(x) is the function.
>
>    Level: beginner
>
> .keywords: SNES, nonlinear, set, function
>
> .seealso: SNESFunction, SNESGetFunction(), SNESComputeFunction(),
> SNESSetJacobian(), SNESSetPicard()
> @*/
> PetscErrorCode  SNESSetFunction(SNES snes,Vec r,PetscErrorCode
> (*SNESFunction)(SNES,Vec,Vec,void*),void *ctx)
> {
>
> Note that since SNESFunction has a manual page the user can click on it to
> see the details of the calling sequence arguments. Thus each manual page
> that uses SNESFunction() doesn't have to duplicate the details of the
> arguments to SNESFunction(). Also search and replace on SNESFunction can be
> used to make changes (yes, I admit not exactly as easy as with a typedef
> but still at least can be done systematically).
>

So this works by convention that we name the argument to match the man page
(e.g., SNESFunction) in all interfaces? The compiler won't help to enforce
that consistency, but it'll deal with the documentation duplication.


>
> Is this a good middle-ground? Something wrong with it?
>
>   Barry
>
> > Cons of typedef:
> > * typedef is less transparent when reading code
> > * information is more scattered (harder for people that don't have the
> source code indexed and don't know where to find definitions)
> > * because every usage does not need to be updated, it's hard to
> determine consequences of a change when reading diff
> >
> > I'll stop using function pointer typedefs if you don't like them.
> >
> > On Fri, Jan 11, 2013 at 10:26 AM, Barry Smith <bsmith at mcs.anl.gov>
> wrote:
> >
> >    The thing I don't like about typedef's for function pointers is that
> they do not display the calling sequencing in the code
> >
> >   For example  from
> >
> > > typedef struct _n_TSDM *TSDM;
> > > struct _n_TSDM {
> > >   TSRHSFunction rhsfunction;
> >
> >   or
> >
> >    PetscErrorCode TSSetRHSFunction(TS ts, TSRHSFunction rhsfunction)
> >
> >   I have no freaking idea what the calling sequence of rhsfunction is.
> but with
> >
> > > typedef struct _n_TSDM *TSDM;
> > > struct _n_TSDM {
> > >   PetscErrorCode (*TSRHSFunction)(TS,PetscReal,Vec,Vec,void*);
> >
> >  or
> >
> >     PetscErrorCode TSSetRHSFunction(TS ts, PetscErrorCode
> (*TSRHSFunction)(TS,PetscReal,Vec,Vec,void*);
> >
> > boom I know exactly the calling sequence.
> >
> >    Maybe there is some way we can do manual pages for these beasties and
> consistency of names in different locations so we get the best of both
> worlds?
> >
> >    Barry
> >
> >
> >
> > On Nov 25, 2012, at 3:58 AM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
> >
> > > On Sun, Nov 25, 2012 at 4:42 AM, Barry Smith <bsmith at mcs.anl.gov>
> wrote:
> > >
> > > typedef struct _n_KSPDM *KSPDM;
> > > struct _n_KSPDM {
> > >   PetscErrorCode (*computeoperators)(KSP,Mat,Mat,MatStructure*,void*);
> > >   PetscErrorCode (*computerhs)(KSP,Vec,void*);
> > >   PetscErrorCode (*computeinitialguess)(KSP,Vec,void*);
> > >
> > > typedef struct _n_SNESDM *SNESDM;
> > > struct _n_SNESDM {
> > >   PetscErrorCode (*computefunction)(SNES,Vec,Vec,void*);
> > >   PetscErrorCode (*computegs)(SNES,Vec,Vec,void*);
> > >   PetscErrorCode
> (*computejacobian)(SNES,Vec,Mat*,Mat*,MatStructure*,void*);
> > >
> > >   /* objective */
> > >   PetscErrorCode (*computeobjective)(SNES,Vec,PetscReal*,void*);
> > >
> > >
> > > typedef struct _n_TSDM *TSDM;
> > > struct _n_TSDM {
> > >   TSRHSFunction rhsfunction;
> > >   TSRHSJacobian rhsjacobian;
> > >
> > >   TSIFunction ifunction;
> > >   TSIJacobian ijacobian;
> > >
> > > The PETSc style guide says  to avoid typedef of function pointers
> unless there is a damn good reason to use them (or it should);
> > >
> > > Can we revisit this choice in the interest of writing one man page
> that documents the assumptions that one can make about a callback routine?
> I hate duplicating the same information in TS{Set,Get}IFunction() and
> DMTS{Set,Get}IFunction(). Also, if a typedef is used everywhere, we can
> more reliably find all places that depend on it. (using grep or M-x
> gtags-find-symbol).
> > >
> > > but why use them for TS but not for SNES or KSP functions?
> > >
> > > Because they were already defined for use with
> TSSetIFunction()/TSGetIFunction().
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20130111/c2427cb1/attachment.html>