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

[Barry Smith]

> On Jan 30, 2015, at 8:53 AM, Dave May <dave.mayhem23 at gmail.com> wrote:
> 
> 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()
> 

   You also need to add a function call interface for turning on/off use of MPIIO.  Perhaps PetscViewerBinarySetUseMPIIO(viewer,PetscBool) and a getter.


> [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.

> 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.

  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
> >
> 
>