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

[Václav Hapla]

28. května 2019 23:46:13 SELČ, "Smith, Barry F." <bsmith at mcs.anl.gov> napsal:
>
>
>> 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?

You can View as many objects as you want (even of various classes) into the same hdf5 file, and they are stored by name, order is unimportant. Yes, I guess Matt is right you need append mode if you don't want to rewrite the file from scratch.

Likewise Load looks up by name and order doesn't matter.

You can also remove things (not implemented in petsc) but the file is actually not reduced in size (actually just links are invalidatef) until you pass it through h5repack which creates a new file with unlinked things omitted.

Vaclav

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