On 27 April 2016 at 22:49, Jed Brown <j...@jedbrown.org> wrote:
> Dave May <dave.mayhe...@gmail.com> writes:
> > This always bugged me.
> > I prefer to access the pointer as at least it's clear what I am doing and
> > when reading the code later, I am not required to ask myself whether the
> DM
> > is actually a shell or not.
> >
> > Why doesn't there exist a generic setter for each object which allows one
> > to set a method for a particular operation?
>
> You're just objecting to the function being named DMShellSetCreateMatrix
> instead of DMSetCreateMatrix?
>
Absolutely!
Its inconsistent with nearly all other PETSc methods.
With all other methods, this function would only take affect if the DM was
of type shell, e.g through using either PetscTryMethod(), or by using
PetscObjectTpeCompare(). This is way it is done in PCShell.
It is also inconsistent with some of the other DMShellSetXXX() as some type
check the type (e.g. DMShellSetCreateMatrix), whilst some do, e.g.
DMShellSetCoarsen().
>
> > The implementation for Mat defines typedef enum { } MatOperation.
> > Using this, we could have
> > PetscErrorCode MatSetOperation(Mat mat,MatOperation op,zzzzz)
> >
> > If there was a similar typedef enum for all other objects, an
> > XXXSetOperation() would be viable.
>
> The MatOperation enums are a bit of a maintenance burden and don't offer
> any type checking. We have them for Mat and Vec (sort of) because there
> are so many methods. We could add them for other objects, but would
> give up type checking relative to the existing specialized functions.
>
Agreed, we shouldn't give up on type checking. But type checking is ignored
with MatShellSetOperation()... This function seems to do approximately what
I proposed. I'm I could autogenerate the functions from the op structure so
it wouldn't be a huge burden (just a lot of code).
Could we at least make the (i) XXXShell objects APIs consistent with each
other in terms of how methods/operations are set, (ii) change all the shell
setter to only take affect if the type was "shell" and (iii) add type
checked setters for Mat, PC and DM (which appear to be the only methods
with shell implementations).
I'd be happy to do this is everyone approves.
Cheers
Dave
