[petsc-dev] [petsc-users] Is PetscViewerBinary special?

[Dave May]

Cool. I'm putting together a pull request for you to look at which does
this re-factoring.



>    You also need to add a function call interface for turning on/off use
> of MPIIO.  Perhaps PetscViewerBinarySetUseMPIIO(viewer,PetscBool) and a
> getter.
>
>
Actually I had this ear-marked for another email.

Since
  PetscViewerBinarySetSkipOptions()
  PetscViewerBinarySetSkipHeader()
all have the args
  PetscViewer viewer, PetscBool skip
I would propose that
  PetscViewerBinarySetMPIIO()
should have the same calling patter, rather than just having the arg
PetscViewer viewer.

PetscViewerBinaryMPIIO() should behave like PetscViewerBinarySkipInfo()
however the former function name is already used - I'll re-name that one to
PetscViewerBinaryReadWriteMPIIO()

I will clean this up and put it all in the fork/branch I've created.


>
> > [5] PetscViewerSetUp_Binary() would be called at the end of
> PetscViewerSetFromOptions_Binary(),
>
>    No it should not be called here. User may want to continue to do stuff
> here.
>

Okay - I'll move it else where.


> > or at the beginning of any of the getters associated with
> PetscViewerBinary, e.g. PetscViewerBinaryGetDescriptor()
>
>
> > and at the beginning of PetscViewerBinaryWrite() PetscViewerBinaryRead().
> >
> > [6] PetscViewerBinaryOpen() would obviously call
> PetscViewerSetFromOptions_Binary().
> >
> > Or does this approach sound like a giant hack?
> > Probably a new operation for the PetscViewer (setup) would be a nicer
> way to go.
>
>    This sounds reasonable to try. Not worth yet adding a setup for all
> viewers.
>
>
Okay, I also thought it wasn't worth the trouble of adding a new operation.

Cheers
  Dave




>   Barry
>
> >
> > I can put this in branch if you think it is worth while pursuing the
> approach I propose.
> >
> > Cheers
> >   Dave
> >
> > On 30 January 2015 at 14:56, Barry Smith <bsmith at mcs.anl.gov> wrote:
> >
> >   Dave,
> >
> >    I did the mpiio hack originally and yes it is very ad hoc and could
> do with a good reorganization. I am fine with accepting a branch that
> treats the MPIIO one as a completely different class; the only reason not
> to do this is if  both classes have essentially the same code but no reuse
> between them.
> >
> >    Barry
> >
> > > On Jan 30, 2015, at 7:51 AM, Dave May <dave.mayhem23 at gmail.com> wrote:
> > >
> > > Thanks Matt.
> > >
> > > Given the PETSc way of doing things, I believe the following should be
> possible:
> > > [1] attach an option prefix to a viewer
> > > [2] skip the header on some binary viewers
> > > [3] use mpiio for some binary viewers, but not all of them.
> > >
> > > However, it doesn't appear to be completely simple to allow this
> behaviour.
> > >
> > > For instance, replacing
> > >     ierr =
> PetscOptionsGetBool(NULL,"-viewer_binary_mpiio",&useMPIIO,NULL);CHKERRQ(ierr);
> > > with
> > >     ierr =
> PetscOptionsGetBool(((PetscObject)viewer)->prefix,"-viewer_binary_mpiio",&useMPIIO,NULL);CHKERRQ(ierr);
> > > won't work as an option prefix will never have been set.
> > >
> > > If I was to add a PetscViewerSetFromOptions_Binary(), I would have to
> ensure that PetscViewerSetFromOptions() was called before
> PetscViewerFileSetName() was called as the latter this triggers a swap of
> the functions used to open binary file, e.g. from
> PetscViewerFileSetName_Binary() to PetscViewerFileSetName_MPIIO()
> > >
> > > Would it be simpler and cleaner to have MPIIO be defined as an
> independent implementation which was a sub-class of the binary class?
> > >
> > > Then we could use command line flags to switch between implementations
> at runtime
> > >   -xxx_viewer_type binary
> > > versus
> > >   -xxx_viewer_type mpiio
> > > rather than having to use
> > >   -xxx_viewer_type binary -viewer_binary_mpiio
> > > which has obvious short comings like forcing all binary viewers,
> independent of the prefix to use mpiio. It would also allow the PetscViewer
> object to be used in a manner which follows the standard PETSc pattern of:
> XXCreate(), XXSetOptionsPrefix(), XXSetType(), XXSetFromOptions().
> > >
> > > Then we could do this
> > >   PetscViewerCreate()
> > >   PetscViewerSetOptionsPrefix()
> > >   PetscViewerSetType()
> > >   PetscViewerFileSetMode()
> > >   PetscViewerBinarySetSkipHeader()
> > >   PetscViewerFileSetName()
> > >   PetscViewerSetFromOptions()
> > > and it wouldn't matter which order SetName(), BinarySkipHeader() etc
> were called.
> > >
> > > What do people think?
> > >
> > >
> > > Cheers
> > >   Dave
> > >
> > >
> > >
> > >
> > >
> > > On 30 January 2015 at 13:56, Matthew Knepley <knepley at gmail.com>
> wrote:
> > > On Fri, Jan 30, 2015 at 2:06 AM, Dave May <dave.mayhem23 at gmail.com>
> wrote:
> > > Hello,
> > >
> > > I've noticed the create function PetscViewerCreate_Binary() doesn't
> appear to be using the options prefix attached to the PetscViewer.
> > > Specifically, I see on line 1276 of
> > >   petsc-3.5.2/src/sys/classes/viewer/impls/binary/binv.c
> > > the following
> > >   ierr =
> PetscOptionsGetBool(NULL,"-viewer_binary_mpiio",&useMPIIO,NULL);CHKERRQ(ierr);
> > >
> > > Is this an oversight or something intentional?
> > >
> > > This is an oversight.
> > >
> > >   Matt
> > >
> > > Are PetscViewers in general behaving such that they cannot be
> configured independently of each other if
> > > PetscViewerSetOptionsPrefix() and PetscViewerSetFromOptions() are
> called?
> > >
> > >
> > > Cheers
> > >   Dave
> > >
> > >
> > >
> > > --
> > > 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
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20150130/f9bf73ba/attachment.html>