<div dir="ltr"><div dir="ltr">On Tue, May 28, 2019 at 5:26 PM Smith, Barry F. via petsc-dev <<a href="mailto:petsc-dev@mcs.anl.gov">petsc-dev@mcs.anl.gov</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
I have concerns. <br>
<br>
Without the use of singletons <br>
<br>
-mat_view binary -vec_view binary<br>
<br>
will result in only the final object viewed being in the file binaryoutput? The idea behind the singletons is they provide continuity for the entire run. <br>
<br>
Note that currently when also creating and destroying the viewers with the same name you only get the most recent stuff. In fact something like <br>
<br>
-mat_view hdf5:myfile <br>
<br>
is likely now buggy since it will only save the most recent matrix viewed in the file the user might expect all the matrices in there.</blockquote><div><br></div><div>you use</div><div><br></div><div> 0mat_view hdf5:myfile::append</div><div><br></div><div><br></div><div> Matt</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Perhaps PetscOptionsGetViewer() needs to register all viewers created with the destroy in PetscFinalize() and never have the user destroy them and attach them to the communicator so they get reused at the next call instead of creating a new one.<br>
<br>
Even if we do the above another problem is<br>
<br>
-xxx_view xx:filename <br>
<br>
will be problematic if there are xxx in different communicators since if we attach them to a communicator we won't find them with a different communicator and will open the file again. I don't know how to fix this.<br>
<br>
I think the entire PetscOptionsGetViewer() business needs a bit more thought. At the moment it is a bit flawed. Making them non-singletons won't fix the flaws.<br>
<br>
> BTW all PETSC_VIEWER_XXX_() manpages say "Creates a XXX PetscViewer shared by all processors in a communicator."<br>
> This is apparently not true - they are singletons stashed to the communicator by MPI_Comm_set_attr() and next time reused if found using MPI_Comm_get_attr(), right?<br>
> So it should be Returns, not Creates.<br>
<br>
This should be fixed in its own PR.<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
> On May 28, 2019, at 10:35 AM, Hapla Vaclav via petsc-dev <<a href="mailto:petsc-dev@mcs.anl.gov" target="_blank">petsc-dev@mcs.anl.gov</a>> wrote:<br>
> <br>
> Let me follow up discussion on creating/controlling viewers from options.<br>
> <br>
> Barry agrees to rename PetscOptionsGetViewer() to PetscViewerCreateFromOptions() in Issue #291.<br>
> <br>
> But now I can see a problem: it mixes Get and Create behavior, as it returns the singleton provided by PETSC_VIEWER_*_() if possible, otherwise creates a new viewer using PetscViewerCreate(). So actually both names PetscOptionsGetViewer() and PetscViewerCreateFromOptions() are confusing.<br>
> <br>
> I think this design is unnecessarily complicated and not really useful.<br>
> I don't see any advantage of reusing the singleton in this context.<br>
> And leads to some IMHO unexpected behavior:<br>
> * -myviewer hdf5:: and -myviewer hdf5 is not the same! I didn't know that but hdf5:: creates a new viewer and hdf5 reuses the singleton.<br>
> * Any properties (including options prefix) set to the viewer returned by PetscOptionsGetViewer() actually affect the singleton and vice versa.<br>
> (See <a href="https://bitbucket.org/petsc/petsc/commits/324f959" rel="noreferrer" target="_blank">https://bitbucket.org/petsc/petsc/commits/324f959</a>)<br>
> <br>
> So in my opinion PetscOptionsGetViewer()<br>
> * should really be renamed PetscViewerCreateFromOptions(),<br>
> * should _always_ return a new instance and call PetscViewerSetFromOptions() on it,<br>
> * could also set the passed prefix to the viewer, i.e. PetscViewerCreateFromOptions(comm, options, "pre_", ...) would result in the viewer having prefix pre_ <br>
> <br>
> Do you think it would break anything?<br>
> <br>
> Thanks,<br>
> Vaclav<br>
> <br>
> BTW all PETSC_VIEWER_XXX_() manpages say "Creates a XXX PetscViewer shared by all processors in a communicator."<br>
> This is apparently not true - they are singletons stashed to the communicator by MPI_Comm_set_attr() and next time reused if found using MPI_Comm_get_attr(), right?<br>
> So it should be Returns, not Creates.<br>
<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>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</div><div><br></div><div><a href="http://www.cse.buffalo.edu/~knepley/" target="_blank">https://www.cse.buffalo.edu/~knepley/</a><br></div></div></div></div></div></div></div></div>