<div dir="ltr"><div><div>Hi Barry,<br><br>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.</div><div><br></div>Cheers<br></div> Dave <br></div><div class="gmail_extra"><br><div class="gmail_quote">On 30 January 2015 at 20:49, Dave May <span dir="ltr"><<a href="mailto:dave.mayhem23@gmail.com" target="_blank">dave.mayhem23@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br>Cool. I'm putting together a pull request for you to look at which does this re-factoring.<br><br><div class="gmail_extra"> <div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span>
</span> You also need to add a function call interface for turning on/off use of MPIIO. Perhaps PetscViewerBinarySetUseMPIIO(viewer,PetscBool) and a getter.<br>
<span><br></span></blockquote><div><br></div></span><div>Actually I had this ear-marked for another email.<br><br></div><div>Since <br> PetscViewerBinarySetSkipOptions()<br> PetscViewerBinarySetSkipHeader()<br>all have the args<br> PetscViewer viewer, PetscBool skip<br></div><div>I would propose that <br> PetscViewerBinarySetMPIIO()<br>should have the same calling patter, rather than just having the arg PetscViewer viewer.</div><div><br>PetscViewerBinaryMPIIO() should behave like PetscViewerBinarySkipInfo()<br></div><div>however the former function name is already used - I'll re-name that one to<br></div><div>PetscViewerBinaryReadWriteMPIIO()</div><div><br>I will clean this up and put it all in the fork/branch I've created.<br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span>
<br>
> [5] PetscViewerSetUp_Binary() would be called at the end of PetscViewerSetFromOptions_Binary(),<br>
<br>
</span> No it should not be called here. User may want to continue to do stuff here.<span><br></span></blockquote><div><br></div></span><div>Okay - I'll move it else where.<br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span>
> or at the beginning of any of the getters associated with PetscViewerBinary, e.g. PetscViewerBinaryGetDescriptor()<br>
<br>
<br>
> and at the beginning of PetscViewerBinaryWrite() PetscViewerBinaryRead().<br>
><br>
> [6] PetscViewerBinaryOpen() would obviously call PetscViewerSetFromOptions_Binary().<br>
><br>
> Or does this approach sound like a giant hack?<br>
> Probably a new operation for the PetscViewer (setup) would be a nicer way to go.<br>
<br>
</span> This sounds reasonable to try. Not worth yet adding a setup for all viewers.<br>
<span><font color="#888888"><br></font></span></blockquote></span><div><br>Okay, I also thought it wasn't worth the trouble of adding a new operation.<br><br>Cheers<span class="HOEnZb"><font color="#888888"><br></font></span></div><span class="HOEnZb"><font color="#888888"><div> Dave<br></div></font></span><div><div class="h5"><div><br><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span><font color="#888888">
Barry<br>
</font></span><div><div><br>
><br>
> I can put this in branch if you think it is worth while pursuing the approach I propose.<br>
><br>
> Cheers<br>
> Dave<br>
><br>
> On 30 January 2015 at 14:56, Barry Smith <<a href="mailto:bsmith@mcs.anl.gov" target="_blank">bsmith@mcs.anl.gov</a>> wrote:<br>
><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>
><br>
> Barry<br>
><br>
> > On Jan 30, 2015, at 7:51 AM, Dave May <<a href="mailto:dave.mayhem23@gmail.com" target="_blank">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" target="_blank">knepley@gmail.com</a>> wrote:<br>
> > On Fri, Jan 30, 2015 at 2:06 AM, Dave May <<a href="mailto:dave.mayhem23@gmail.com" target="_blank">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>
><br>
<br>
</div></div></blockquote></div></div></div></div></div>
</blockquote></div><br></div>