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

[Dave May]

Hi Barry,

Okay - fair enough. Almost of all the code would be re-used with my
suggestion.

I hope you don't mind, I moved this discussion to the dev list as I wanted
to propose a solution.

What about this as an alternative?

[1] I add PetscViewerSetFromOptions_Binary()

[2] I add a setupcalled flag into PetscViewer_Binary

[3] I modify PetscViewerFileSetName_Binary() to just set the file name
string and defer actually opening of the file to within in a function
PetscViewerSetUp_Binary().
Depending on whether mpiio is set we call (essentially) the current
contents of PetscViewerFileSetName_MPIIO() or
PetscViewerFileSetName_Binary().
However the parsing of options -viewer_binary_skip_info etc would be moved
into PetscViewerSetFromOptions_Binary()

[4] I modify the PetscViewerDestory_Binary() such that it calls the
appropriate functions for mpiio or non-mpiio and remove
PetscObjectComposeFunction() from PetscViewerBinarySetMPIIO_Binary()

[5] PetscViewerSetUp_Binary() would be called at the end of
PetscViewerSetFromOptions_Binary(), 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.

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/c14d057d/attachment.html>