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

[Dave May]

Hi Barry,

Following up on this email thread, I've created a pull-request which
implements the idea I proposed in relation to refactoring the
PetscViewerBinary code.

Cheers
  Dave

On 30 January 2015 at 20:49, Dave May <dave.mayhem23 at gmail.com> wrote:

>
> 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/20150209/23c4947e/attachment.html>