[petsc-dev] PETSC_VIEWER_*_() vs PetscOptionsGetViewer()

[Matthew Knepley]

On Tue, May 28, 2019 at 5:46 PM Smith, Barry F. <bsmith at mcs.anl.gov> wrote:

>
>
> > On May 28, 2019, at 4:40 PM, Matthew Knepley <knepley at gmail.com> wrote:
> >
> > On Tue, May 28, 2019 at 5:26 PM Smith, Barry F. via petsc-dev <
> petsc-dev at mcs.anl.gov> wrote:
> >
> >   I have concerns.
> >
> >   Without the use of singletons
> >
> >    -mat_view binary -vec_view binary
> >
> >   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.
> >
> >   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
> >
> >    -mat_view hdf5:myfile
> >
> >   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.
> >
> > you use
> >
> >   0mat_view hdf5:myfile::append
>
>    And all the crap from yesterday's runs are also in the file?
>

Barry,

  -mat_view binary -vec_view binary:::append

    Matt


> >
> >
> >    Matt
> >
> > 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.
> >
> >   Even if we do the above another problem is
> >
> >    -xxx_view xx:filename
> >
> > 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.
> >
> >   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.
> >
> > > BTW all PETSC_VIEWER_XXX_() manpages say "Creates a XXX PetscViewer
> shared by all processors in a communicator."
> > > 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?
> > > So it should be Returns, not Creates.
> >
> >   This should be fixed in its own PR.
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > > On May 28, 2019, at 10:35 AM, Hapla Vaclav via petsc-dev <
> petsc-dev at mcs.anl.gov> wrote:
> > >
> > > Let me follow up discussion on creating/controlling viewers from
> options.
> > >
> > > Barry agrees to rename PetscOptionsGetViewer() to
> PetscViewerCreateFromOptions() in Issue #291.
> > >
> > > 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.
> > >
> > > I think this design is unnecessarily complicated and not really useful.
> > > I don't see any advantage of reusing the singleton in this context.
> > > And leads to some IMHO unexpected behavior:
> > > * -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.
> > > * Any properties (including options prefix) set to the viewer returned
> by PetscOptionsGetViewer() actually affect the singleton and vice versa.
> > >   (See https://bitbucket.org/petsc/petsc/commits/324f959)
> > >
> > > So in my opinion PetscOptionsGetViewer()
> > > * should really be renamed PetscViewerCreateFromOptions(),
> > > * should _always_ return a new instance and call
> PetscViewerSetFromOptions() on it,
> > > * could also set the passed prefix to the viewer, i.e.
> PetscViewerCreateFromOptions(comm, options, "pre_", ...) would result in
> the viewer having prefix pre_
> > >
> > > Do you think it would break anything?
> > >
> > > Thanks,
> > > Vaclav
> > >
> > > BTW all PETSC_VIEWER_XXX_() manpages say "Creates a XXX PetscViewer
> shared by all processors in a communicator."
> > > 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?
> > > So it should be Returns, not Creates.
> >
> >
> >
> > --
> > 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
> >
> > https://www.cse.buffalo.edu/~knepley/
>
>

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

https://www.cse.buffalo.edu/~knepley/ <http://www.cse.buffalo.edu/~knepley/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20190528/ec356f08/attachment.html>