<div dir="ltr"><div><div><div><div><div><div><div>Hi Barry,<br><br>Okay - fair enough. Almost of all the code would be re-used with my suggestion.<br><br></div><div>I hope you don't mind, I moved this discussion to the dev list as I wanted to propose a solution.<br></div><div><br></div>What about this as an alternative?<br></div><br>[1] I add PetscViewerSetFromOptions_Binary()<br></div><br>[2] I add a setupcalled flag into PetscViewer_Binary<br></div><br>[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(). <br>Depending on whether mpiio is set we call (essentially) the current contents of PetscViewerFileSetName_MPIIO() or PetscViewerFileSetName_Binary(). <br>However the parsing of options -viewer_binary_skip_info etc would be moved into PetscViewerSetFromOptions_Binary()<br><br></div>[4] I modify the PetscViewerDestory_Binary() such that it calls the appropriate functions for mpiio or non-mpiio and remove PetscObjectComposeFunction() from PetscViewerBinarySetMPIIO_Binary()<br><br>[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().<br><br>[6] PetscViewerBinaryOpen() would obviously call PetscViewerSetFromOptions_Binary().<br><br></div>Or does this approach sound like a giant hack?<br></div><div><div><div><div><div><div>Probably a new operation for the PetscViewer (setup) would be a nicer way to go.<br><br></div><div>I can put this in branch if you think it is worth while pursuing the approach I propose.<br></div><div><br></div><div>Cheers<br></div><div> Dave<br></div></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 30 January 2015 at 14:56, Barry Smith <span dir="ltr"><<a href="mailto:bsmith@mcs.anl.gov" target="_blank">bsmith@mcs.anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Dave,<br>
<br>
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.<br>
<span class=""><font color="#888888"><br>
Barry<br>
</font></span><div class=""><div class="h5"><br>
> On Jan 30, 2015, at 7:51 AM, Dave May <<a href="mailto:dave.mayhem23@gmail.com">dave.mayhem23@gmail.com</a>> wrote:<br>
><br>
> Thanks Matt.<br>
><br>
> Given the PETSc way of doing things, I believe the following should be possible:<br>
> [1] attach an option prefix to a viewer<br>
> [2] skip the header on some binary viewers<br>
> [3] use mpiio for some binary viewers, but not all of them.<br>
><br>
> However, it doesn't appear to be completely simple to allow this behaviour.<br>
><br>
> For instance, replacing<br>
> ierr = PetscOptionsGetBool(NULL,"-viewer_binary_mpiio",&useMPIIO,NULL);CHKERRQ(ierr);<br>
> with<br>
> ierr = PetscOptionsGetBool(((PetscObject)viewer)->prefix,"-viewer_binary_mpiio",&useMPIIO,NULL);CHKERRQ(ierr);<br>
> won't work as an option prefix will never have been set.<br>
><br>
> 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()<br>
><br>
> Would it be simpler and cleaner to have MPIIO be defined as an independent implementation which was a sub-class of the binary class?<br>
><br>
> Then we could use command line flags to switch between implementations at runtime<br>
> -xxx_viewer_type binary<br>
> versus<br>
> -xxx_viewer_type mpiio<br>
> rather than having to use<br>
> -xxx_viewer_type binary -viewer_binary_mpiio<br>
> 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().<br>
><br>
> Then we could do this<br>
> PetscViewerCreate()<br>
> PetscViewerSetOptionsPrefix()<br>
> PetscViewerSetType()<br>
> PetscViewerFileSetMode()<br>
> PetscViewerBinarySetSkipHeader()<br>
> PetscViewerFileSetName()<br>
> PetscViewerSetFromOptions()<br>
> and it wouldn't matter which order SetName(), BinarySkipHeader() etc were called.<br>
><br>
> What do people think?<br>
><br>
><br>
> Cheers<br>
> Dave<br>
><br>
><br>
><br>
><br>
><br>
> On 30 January 2015 at 13:56, Matthew Knepley <<a href="mailto:knepley@gmail.com">knepley@gmail.com</a>> wrote:<br>
> On Fri, Jan 30, 2015 at 2:06 AM, Dave May <<a href="mailto:dave.mayhem23@gmail.com">dave.mayhem23@gmail.com</a>> wrote:<br>
> Hello,<br>
><br>
> I've noticed the create function PetscViewerCreate_Binary() doesn't appear to be using the options prefix attached to the PetscViewer.<br>
> Specifically, I see on line 1276 of<br>
> petsc-3.5.2/src/sys/classes/viewer/impls/binary/binv.c<br>
> the following<br>
> ierr = PetscOptionsGetBool(NULL,"-viewer_binary_mpiio",&useMPIIO,NULL);CHKERRQ(ierr);<br>
><br>
> Is this an oversight or something intentional?<br>
><br>
> This is an oversight.<br>
><br>
> Matt<br>
><br>
> Are PetscViewers in general behaving such that they cannot be configured independently of each other if<br>
> PetscViewerSetOptionsPrefix() and PetscViewerSetFromOptions() are called?<br>
><br>
><br>
> Cheers<br>
> Dave<br>
><br>
><br>
><br>
> --<br>
> What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead.<br>
> -- Norbert Wiener<br>
><br>
<br>
</div></div></blockquote></div><br></div></div></div>