On Jan 30, 2015, at 8:53 AM, Dave May <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote: On Fri, Jan 30, 2015 at 2:06 AM, Dave May <[email protected]> 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