VecLoadIntoVector and double dispatch

2009-12-05 Thread Jed Brown
On Fri, 4 Dec 2009 18:29:19 -0600, Barry Smith  wrote:
>  This may be crux of the current discussion.

This part was actually orthogonal to the extensible double dispatch
issue which was:

  It should be possible for VecView(X,V) to invoke a third-party viewer
  V even when X (including the DM associated with it) has never come in
  contact with code that is aware of the implementation of V.

So there is no opportunity to modify vec->ops.  I'm thinking of
dynamically loading PetscViewerCreate_Foo, and selecting it with
-viewer_type foo.  Every other PETSc object has this ability,
dynamically loaded implementations are first-class, they certainly don't
require modifying other objects to be functional.  But they only need
single-dispatch, extensible double dispatch is hard and every solution
involves compomises.  I'm just frustrated with the current compromise,
maybe it would be better if the language made the choice for us by
offering native multimethods.


Dispatch through the DM has no effect on this problem, it only avoids
having a Vec with the same type name, but different type (because it has
different methods) and "viewnative" being a special case, and it's not a
big deal to me.

Jed



VecLoadIntoVector and double dispatch

2009-12-04 Thread Barry Smith

On Dec 4, 2009, at 10:31 AM, Jed Brown wrote:

> On Fri, 4 Dec 2009 08:52:44 -0600, Barry Smith   
> wrote:
>> This is not accurate. The SAMRAI vector class does not implement
>> it. Yes, this means the SAMRAI vector class cannot use any PETSc  
>> built
>> in matrix classes, but that is ok it provides its own.
>
> Right, so I would get an error if I tried to use my viewer with them,
> that's the same as if I tried MatMult with a PETSc matrix.  But we  
> don't
> have to go edit src/vec/vec any time we put something new in mat/ 
> impls.

 This may be crux of the current discussion. You state that if one  
wants to have a vecview method attached to a Vec that does stuff  
specificly related to a particular DM then one needs to edit something  
in src/vec/vec and THAT IS VERY BAD. I totally agree with you that is  
very bad, but my point is that one does not need to edit anything in  
src/vec/vec. For example, the parallel vectors created with  
DACreateGlobalVector() have a new vecview() method that stores the Vec  
in a different way then the default (converting to global natural  
ordering). This is done by defining the method VecView_MPI_DA() with  
the DA definition and then stuck into the function pointer, this is  
done in DA directory, not the vec directorty. In inheritence language  
a DA Vec is inherited off a regular Vec.

Thus I do not understand your whole business of DMVecView(). No  
need for this, just change the VecView() for your DM vectors to do  
whatever is needed for your DM. No editing of src/vec/vec code needed.

Barry

>
>> (note also that PETSc allows incomplete implementations of abstract
>> classes where a new class implementaiton simply does not provide some
>> methods and errors are generated if one tries to use them; I think
>> this is a great design feature because it allows many-method base
>> abstract classes, but I realize most of the computing world would be
>> shocked and disgusted by this design feature).
>
> I also think it is good, at least for this problem domain.  Especially
> if the alternative looks anything like Trilinos/Epetra.
>
>>To the algebraic solvers I agree a " Vec is just an element of
>> some finite dimensional ." (since that is all the solvers need to
>> know about them).  But to a specific application a Vec can be thought
>> of as the linear algebra beast PLUS all kinds of grid information etc
>> (like the DM).  So I think of a (application) Vec in the application
>> as basically derived off of two classes:  the linear algebra beast
>> PLUS a DM (it is implemented as a isA(linear algebra beast) and
>> hasA(DM beast)).
>
> Okay, so most DM-level operations take the DM as an explicit argument.
> Since the Vec holds a reference to it's DM, this explicit argument
> appears to be redundant.  It's a bit of a knowledge loop because src/ 
> vec
> is not allowed to know about the DM, but there are other contexts  
> where
> Vec has a DM.
>
> Hypothetically now, what if all the Vec functionality that really  
> needed
> to reference the DM was actually implemented as
>
>  VecGetDM(vec,&dm);
>  DMDoSomethingWithVec(dm,vec,...);
>
> When the vector has no context, it would be calling a "null" DM that
> just represented plain Cartesian space.  This would avoid
> monkey-patching Vec to get control from the purely algebraic world to
> src/dm, but it would mean that src/vec could no longer be completely
> oblivious to the existence of DM.
>
> Similar to MatMAIJRedimension, you could reinterpret the vector in any
> isometric space.  This would achieve something similar to the
> native/super distinction without limiting to just two options, and
> without src/vec having any knowledge of "viewer format".  The  
> difference
> that now Vec output including the DM is a property of the Vec rather
> than the viewer.
>
>>   Regarding where code needs to be changed/where : The reason for the
>> "native/super" support for the VecLoadIntoVector() (which I admit is
>> terribly crude and ugly) is exactly so that one can implement a new
>> loader/writer directly for a specific Vec/DM combination IN THE
>> APPLICATION CODE or another library.
>
> There is a false separation here, the viewer can only be used if the  
> Vec
> is created in a context that knows about the viewer.  It can't be used
> if VecView is the first time that code aware of the viewer comes in
> contact with the Vec.
>
>> Do you have suggestions on how one could improve the "native/super"
>> thing to satisfy you?
>
> Hmm, so I'm not sure it would be terrible for VecView to always  
> dispatch
> through DMVecView (which may call back into src/vec to move the  
> bytes in
> the underlying algebraic representation).  It's still not enough to
> enable a third-party viewer that works with existing DMs (assuming the
> viewer never has a chance to patch the DMs before being called).
>
> For a completely autonomous/symmetric dispatch, there could be a  
> global
> table
>
>  [(DMType, ViewerType),

VecLoadIntoVector and double dispatch

2009-12-04 Thread Jed Brown
On Fri, 4 Dec 2009 08:52:44 -0600, Barry Smith  wrote:
>  This is not accurate. The SAMRAI vector class does not implement  
> it. Yes, this means the SAMRAI vector class cannot use any PETSc built  
> in matrix classes, but that is ok it provides its own.

Right, so I would get an error if I tried to use my viewer with them,
that's the same as if I tried MatMult with a PETSc matrix.  But we don't
have to go edit src/vec/vec any time we put something new in mat/impls.

> (note also that PETSc allows incomplete implementations of abstract
> classes where a new class implementaiton simply does not provide some
> methods and errors are generated if one tries to use them; I think
> this is a great design feature because it allows many-method base
> abstract classes, but I realize most of the computing world would be
> shocked and disgusted by this design feature).

I also think it is good, at least for this problem domain.  Especially
if the alternative looks anything like Trilinos/Epetra.

> To the algebraic solvers I agree a " Vec is just an element of  
> some finite dimensional ." (since that is all the solvers need to  
> know about them).  But to a specific application a Vec can be thought  
> of as the linear algebra beast PLUS all kinds of grid information etc  
> (like the DM).  So I think of a (application) Vec in the application  
> as basically derived off of two classes:  the linear algebra beast  
> PLUS a DM (it is implemented as a isA(linear algebra beast) and  
> hasA(DM beast)).

Okay, so most DM-level operations take the DM as an explicit argument.
Since the Vec holds a reference to it's DM, this explicit argument
appears to be redundant.  It's a bit of a knowledge loop because src/vec
is not allowed to know about the DM, but there are other contexts where
Vec has a DM.

Hypothetically now, what if all the Vec functionality that really needed
to reference the DM was actually implemented as

  VecGetDM(vec,&dm);
  DMDoSomethingWithVec(dm,vec,...);

When the vector has no context, it would be calling a "null" DM that
just represented plain Cartesian space.  This would avoid
monkey-patching Vec to get control from the purely algebraic world to
src/dm, but it would mean that src/vec could no longer be completely
oblivious to the existence of DM.

Similar to MatMAIJRedimension, you could reinterpret the vector in any
isometric space.  This would achieve something similar to the
native/super distinction without limiting to just two options, and
without src/vec having any knowledge of "viewer format".  The difference
that now Vec output including the DM is a property of the Vec rather
than the viewer.

>Regarding where code needs to be changed/where : The reason for the
> "native/super" support for the VecLoadIntoVector() (which I admit is
> terribly crude and ugly) is exactly so that one can implement a new
> loader/writer directly for a specific Vec/DM combination IN THE
> APPLICATION CODE or another library.

There is a false separation here, the viewer can only be used if the Vec
is created in a context that knows about the viewer.  It can't be used
if VecView is the first time that code aware of the viewer comes in
contact with the Vec.

> Do you have suggestions on how one could improve the "native/super"
> thing to satisfy you?

Hmm, so I'm not sure it would be terrible for VecView to always dispatch
through DMVecView (which may call back into src/vec to move the bytes in
the underlying algebraic representation).  It's still not enough to
enable a third-party viewer that works with existing DMs (assuming the
viewer never has a chance to patch the DMs before being called).

For a completely autonomous/symmetric dispatch, there could be a global
table

  [(DMType, ViewerType), function]

As long as this table was dynamic, any number of viewers, DMs, could
register methods in the table.  Holes in the table could be filled in
from third-party sources.  It would not require patching the object you
want to view (actually it would prevent patching objects so that they
viewed differently without changing the type of the object).

> So it disturbs me that a method in a Viewer would even know what a
> Vec. It leads to these loops of knowledge that make code much more
> complicated. I would do anything to prevent these loops of knowledge I
> consider them so troublesome.

I understand this and it bugs me, but I'm having trouble finding a place
for another object that knows about both.  *Something* needs to be able
to wire up and interpret the file, some of the logic is currently with
my DMView, but the stuff that doesn't belong there is in the Viewer.

Jed



VecLoadIntoVector and double dispatch

2009-12-04 Thread Jed Brown
On Thu, 3 Dec 2009 16:40:10 -0600, Barry Smith  wrote:
> The vector has a pointer to the DM so the VecView() for that  
> derived vector class has access to the DM information.  The same  
> viewer object can be used with a bunch of different sized Vecs since  
> it gets the DM information out of each vector. With your model when  
> the DM info is attached to the viewer you need to pass in a different  
> viewer for different vectors (of different sizes). This is just plan  
> nuts.

The DM info is not attached to the viewer, I have plain Vecs with a DM
composed so it can call DMView as in your snippet below.  The viewer
holds no state with regard to the DM.  But VecView_MyViewer_DM needs to
get called somehow, it will in turn call DMView and use an internal
Viewer API so that the file has a connection from the Vec to the DM.

> I say this is nonsense, it ain't more closely coupled to the viewer.

VecGetArray is public, all vectors have to implement it in order to
function in PETSc.  The internal Viewer API to write the bytes of a Vec
with some connection to a DM is not public in the sense that users never
call it directly and no other viewers implement it.  You can substitute
any other Vec and this code will still write it correctly because your
Vec would be broken if it didn't implement VecGetArray [1].

DM currently monkey-patches Vec to set ops->view and
ops->loadintovector, but it doesn't rely on the implementation of Vec
(the definition of Vec_Seq/MPI is never used in src/dm) and in principle
DACreateGlobalVector could call VecSetFromOptions so that it's actually
operating with any Vec type (e.g. the Vec could implement it's
operations on a GPU, only bringing the values to the CPU for
VecGetArray).  Many viewers (formats) have no special representation of
vectors that come from a DM [2] which is the reason for having
ops->viewnative and ops->loadintovectornative.

> >  But somehow, calling VecView needs to be able to call the viewer
> > with enough context for the viewer to wire up the metadata.  Note
> > that the coordinates of the nodes in the DM are part of the vector
> > (it's an ALE formulation) so the metadata is nontrivial.
> 
> Each vector has a pointer to its DM (DMComposite etc) so its  
> viewer can dump/read that info when needed. In fact, when writing a  
> vector it could do something like
> 
> myvecview(Vec v,Viewer viewer)
> {
>  DM dm = getreferencetoDMfrom(v);
>  DMView(dm,viewer);
>  ... now dump my vector with writes or whatever

My code looks exactly like this, my point is that the elided part
involves only the public Vec API, but a private Viewer API.  I can put
this chunk of code anywhere, but it needs changes when I make private
changes to the viewer, it doesn't need anything when I make private
changes to the DM or Vec (it doesn't even need to know the types of
those objects).  It is currently sitting with my DM, mostly because I
wrote the DMView first.

> You have to admit that it would be crazy to have the DM viewer method
> stored inside a viewer object which seems to be what your model is.

I wish I could get the dispatch with a symmetric relationship, but in
lieu of that, I don't especially care where the function pointer is,
what matters is that there is a way to get DMView_MyViewer_YourDM
called [3].  As long as dispatch is happening via PetscTypeCompare against
all the known types, we are restricted in that we either have no way to
use a third-party Viewer, or no way to view a third-party DM.  The
current choice is the former; I don't think it's inherently superior,
it's just a choice of which component can be extended without modifying
every object it touches.

If I was distributing a binary-only viewer plugin, I would have no
choice but to put all these methods in with the viewer (but I would
currently have no way to get them called).  Note that a viewer can be
very functional without ever using a private API or looking underneath
obj->data.  With all other PETSc objects (DM not so much because nobody
works with the generic DM interface), I can distribute a binary-only
implementation which is first class after dynamic loading.

My DMView is packaged up with my DM because it needs to know many
internal details of the DM, but the higher level logic about how to
write a time-dependent file with association between Vecs and DMs and
possibly other objects is currently with my viewer.  Now I need to write
DMView_MyViewer_DMComposite, but I won't be able to get it called
without modifying DMComposite (not okay since my viewer is not part of
PETSc).

> You know my answer. The Viewers are abstractions of file IO, ;  
> except instead of having only methods like read()/write() bytes it has  
> somewhat "higher level" operations. Note: these methods can include  
> graphics operations for example that are still essentially like  
> writes(). YOU are making the viewers into these strange "all knowing"  
> beasts which I think is wrong.

Current PETS

VecLoadIntoVector and double dispatch

2009-12-04 Thread Matthew Knepley
On Fri, Dec 4, 2009 at 8:52 AM, Barry Smith  wrote:

>
> On Dec 4, 2009, at 2:58 AM, Jed Brown wrote:
>
>  On Thu, 3 Dec 2009 16:40:10 -0600, Barry Smith 
>> wrote:
>>
>>>   The vector has a pointer to the DM so the VecView() for that
>>> derived vector class has access to the DM information.  The same
>>> viewer object can be used with a bunch of different sized Vecs since
>>> it gets the DM information out of each vector. With your model when
>>> the DM info is attached to the viewer you need to pass in a different
>>> viewer for different vectors (of different sizes). This is just plan
>>> nuts.
>>>
>>
>> The DM info is not attached to the viewer, I have plain Vecs with a DM
>> composed so it can call DMView as in your snippet below.  The viewer
>> holds no state with regard to the DM.  But VecView_MyViewer_DM needs to
>> get called somehow,
>>
>
>Agreed.
>
>
>  it will in turn call DMView and use an internal
>> Viewer API so that the file has a connection from the Vec to the DM.
>>
>>I say this is nonsense, it ain't more closely coupled to the viewer.
>>>
>>
>> VecGetArray is public, all vectors have to implement it in order to
>> function in PETSc.
>>
>
>This is not accurate. The SAMRAI vector class does not implement it.
> Yes, this means the SAMRAI vector class cannot use any PETSc built in matrix
> classes, but that is ok it provides its own. (note also that PETSc allows
> incomplete implementations of abstract classes where a new class
> implementaiton simply does not provide some methods and errors are generated
> if one tries to use them; I think this is a great design feature because it
> allows many-method base abstract classes, but I realize most of the
> computing world would be shocked and disgusted by this design feature).
>
>
>
>  The internal Viewer API to write the bytes of a Vec
>> with some connection to a DM is not public in the sense that users never
>> call it directly and no other viewers implement it.  You can substitute
>> any other Vec and this code will still write it correctly because your
>> Vec would be broken if it didn't implement VecGetArray [1].
>>
>> DM currently monkey-patches Vec to set ops->view and
>> ops->loadintovector, but it doesn't rely on the implementation of Vec
>> (the definition of Vec_Seq/MPI is never used in src/dm) and in principle
>> DACreateGlobalVector could call VecSetFromOptions so that it's actually
>> operating with any Vec type (e.g. the Vec could implement it's
>> operations on a GPU, only bringing the values to the CPU for
>> VecGetArray).  Many viewers (formats) have no special representation of
>> vectors that come from a DM [2] which is the reason for having
>> ops->viewnative and ops->loadintovectornative.
>>
>>  But somehow, calling VecView needs to be able to call the viewer
 with enough context for the viewer to wire up the metadata.  Note
 that the coordinates of the nodes in the DM are part of the vector
 (it's an ALE formulation) so the metadata is nontrivial.

>>>
>>>   Each vector has a pointer to its DM (DMComposite etc) so its
>>> viewer can dump/read that info when needed. In fact, when writing a
>>> vector it could do something like
>>>
>>> myvecview(Vec v,Viewer viewer)
>>> {
>>>DM dm = getreferencetoDMfrom(v);
>>>DMView(dm,viewer);
>>>... now dump my vector with writes or whatever
>>>
>>
>> My code looks exactly like this, my point is that the elided part
>> involves only the public Vec API, but a private Viewer API.  I can put
>> this chunk of code anywhere, but it needs changes when I make private
>> changes to the viewer, it doesn't need anything when I make private
>> changes to the DM or Vec (it doesn't even need to know the types of
>> those objects).  It is currently sitting with my DM, mostly because I
>> wrote the DMView first.
>>
>>  You have to admit that it would be crazy to have the DM viewer method
>>> stored inside a viewer object which seems to be what your model is.
>>>
>>
>> I wish I could get the dispatch with a symmetric relationship, but in
>> lieu of that, I don't especially care where the function pointer is,
>> what matters is that there is a way to get DMView_MyViewer_YourDM
>> called [3].  As long as dispatch is happening via PetscTypeCompare against
>> all the known types, we are restricted in that we either have no way to
>> use a third-party Viewer, or no way to view a third-party DM.  The
>> current choice is the former; I don't think it's inherently superior,
>> it's just a choice of which component can be extended without modifying
>> every object it touches.
>>
>> If I was distributing a binary-only viewer plugin, I would have no
>> choice but to put all these methods in with the viewer (but I would
>> currently have no way to get them called).  Note that a viewer can be
>> very functional without ever using a private API or looking underneath
>> obj->data.  With all other PETSc objects (DM not so much because nobody
>> works with the generic DM interfa

VecLoadIntoVector and double dispatch

2009-12-04 Thread Barry Smith

On Dec 4, 2009, at 2:58 AM, Jed Brown wrote:

> On Thu, 3 Dec 2009 16:40:10 -0600, Barry Smith   
> wrote:
>>The vector has a pointer to the DM so the VecView() for that
>> derived vector class has access to the DM information.  The same
>> viewer object can be used with a bunch of different sized Vecs since
>> it gets the DM information out of each vector. With your model when
>> the DM info is attached to the viewer you need to pass in a different
>> viewer for different vectors (of different sizes). This is just plan
>> nuts.
>
> The DM info is not attached to the viewer, I have plain Vecs with a DM
> composed so it can call DMView as in your snippet below.  The viewer
> holds no state with regard to the DM.  But VecView_MyViewer_DM needs  
> to
> get called somehow,

 Agreed.

> it will in turn call DMView and use an internal
> Viewer API so that the file has a connection from the Vec to the DM.
>
>>I say this is nonsense, it ain't more closely coupled to the  
>> viewer.
>
> VecGetArray is public, all vectors have to implement it in order to
> function in PETSc.

 This is not accurate. The SAMRAI vector class does not implement  
it. Yes, this means the SAMRAI vector class cannot use any PETSc built  
in matrix classes, but that is ok it provides its own. (note also that  
PETSc allows incomplete implementations of abstract classes where a  
new class implementaiton simply does not provide some methods and  
errors are generated if one tries to use them; I think this is a great  
design feature because it allows many-method base abstract classes,  
but I realize most of the computing world would be shocked and  
disgusted by this design feature).


> The internal Viewer API to write the bytes of a Vec
> with some connection to a DM is not public in the sense that users  
> never
> call it directly and no other viewers implement it.  You can  
> substitute
> any other Vec and this code will still write it correctly because your
> Vec would be broken if it didn't implement VecGetArray [1].
>
> DM currently monkey-patches Vec to set ops->view and
> ops->loadintovector, but it doesn't rely on the implementation of Vec
> (the definition of Vec_Seq/MPI is never used in src/dm) and in  
> principle
> DACreateGlobalVector could call VecSetFromOptions so that it's  
> actually
> operating with any Vec type (e.g. the Vec could implement it's
> operations on a GPU, only bringing the values to the CPU for
> VecGetArray).  Many viewers (formats) have no special representation  
> of
> vectors that come from a DM [2] which is the reason for having
> ops->viewnative and ops->loadintovectornative.
>
>>> But somehow, calling VecView needs to be able to call the viewer
>>> with enough context for the viewer to wire up the metadata.  Note
>>> that the coordinates of the nodes in the DM are part of the vector
>>> (it's an ALE formulation) so the metadata is nontrivial.
>>
>>Each vector has a pointer to its DM (DMComposite etc) so its
>> viewer can dump/read that info when needed. In fact, when writing a
>> vector it could do something like
>>
>> myvecview(Vec v,Viewer viewer)
>> {
>> DM dm = getreferencetoDMfrom(v);
>> DMView(dm,viewer);
>> ... now dump my vector with writes or whatever
>
> My code looks exactly like this, my point is that the elided part
> involves only the public Vec API, but a private Viewer API.  I can put
> this chunk of code anywhere, but it needs changes when I make private
> changes to the viewer, it doesn't need anything when I make private
> changes to the DM or Vec (it doesn't even need to know the types of
> those objects).  It is currently sitting with my DM, mostly because I
> wrote the DMView first.
>
>> You have to admit that it would be crazy to have the DM viewer method
>> stored inside a viewer object which seems to be what your model is.
>
> I wish I could get the dispatch with a symmetric relationship, but in
> lieu of that, I don't especially care where the function pointer is,
> what matters is that there is a way to get DMView_MyViewer_YourDM
> called [3].  As long as dispatch is happening via PetscTypeCompare  
> against
> all the known types, we are restricted in that we either have no way  
> to
> use a third-party Viewer, or no way to view a third-party DM.  The
> current choice is the former; I don't think it's inherently superior,
> it's just a choice of which component can be extended without  
> modifying
> every object it touches.
>
> If I was distributing a binary-only viewer plugin, I would have no
> choice but to put all these methods in with the viewer (but I would
> currently have no way to get them called).  Note that a viewer can be
> very functional without ever using a private API or looking underneath
> obj->data.  With all other PETSc objects (DM not so much because  
> nobody
> works with the generic DM interface), I can distribute a binary-only
> implementation which is first class after dynamic loading.
>
> My DMView is packa

VecLoadIntoVector and double dispatch

2009-12-03 Thread Jed Brown
On Thu, 3 Dec 2009 15:09:03 -0600, Barry Smith  wrote:
> But then your viewer must know about DM? That means that your  
> viewer knows about the internal structure of the vector?

Actually, it doesn't use any internal knowledge about the vector, but it
does know about the DM (which has metadata like connectivity).

> I think this is very wrong. The viewer object is a lower level  
> object, it doesn't even know about linear algebra or vectors, nothing.  
> It should only expose a bunch of methods that the "higher level"  
> objects use. No viewer class show know about any Vec class or even the  
> existence of the Vec abstract class, but Vec classes can know about  
> the abstract Viewer class and even specific viewer classes.

I'm struggling with this and the "loops" are troubling.

So in a common scenario, we have a vector based on DMComposite which is
composed of, say, one DA and one unstructured DM (I call it FS).  It's
the TS that will be making the call to write model state out at special
time steps and will need to read the state back later in the adjoint
model.  The TS cannot know anything about the DM or implementation
details of the Vec, therefore it is only allowed to call VecView,
VecLoadIntoVector, and some viewer function to find time steps.

So there needs to be some chunk of code that knows how to wire up this
file with appropriate metadata, and some other chunks that can write
Vecs and DMs wherever the Viewer tells it to.  The first clearly belongs
with the Viewer, the latter doesn't clearly (to me) belong on either
side.  Since it only uses the VecGetArray level of information about the
vec, but uses an internal Viewer API, it seems more closely coupled to
the viewer.  But somehow, calling VecView needs to be able to call the
viewer with enough context for the viewer to wire up the metadata.  Note
that the coordinates of the nodes in the DM are part of the vector (it's
an ALE formulation) so the metadata is nontrivial.

In this case, I have a new viewer so having dispatch in the other
objects is constraining because I would have to go modify every one that
I want my viewer to work with.  But if I was implementing a new Vec, I
wouldn't want the viewer to be in charge of dispatch because then I'd
have to go modify those viewers.

I'd love to hear your suggestions.


Jed



VecLoadIntoVector and double dispatch

2009-12-03 Thread Jed Brown
Thanks for the explanation.

On Thu, 3 Dec 2009 13:48:21 -0600, Barry Smith  wrote:
> This is wrong. What about a binary viewer method for the PETSc Vec  
> class implemented in SAMRAI? This definitely cannot rely on  
> VecGetArray() and belongs with the Vec class not the viewer classes  
> (which know nothing about SAMRAI even existing.

I don't know what is in SAMRAI's Vec, but how about my HDF5 viewer that
encodes information about the DM?  The Vec doesn't know anything about
my viewer existing.  Once we have more than one implementation of Viewer
and more than one of Vec, no asymmetric solution can be perfect.  I can
live with static dispatch or other hacks for now.

Jed



VecLoadIntoVector and double dispatch

2009-12-03 Thread Jed Brown
I'm sort of confused about vec->ops->loadintovectornative, this seems to
just be a way to let users provide *one* custom viewer on a particular
vector without breaking PETSc's own viewers.  But it really doesn't
provide a reasonably solution for an intermediate library, or user code
with multiple viewers.

Extensible double dispatch is not something C is heralded for, but it's
not impossible.  An easy-to-code approach is to just do it all with
PetscObjectFunctionComposeDynamic.  The FList could get a bit long, but
I think you'd be hard-pressed to find a real code where dispatch time
for VecLoadIntoVector and (a couple of other viewer-related functions)
was an issue.  For best performance, PetscObject could acquire a field
for "type index" or some such (e.g. managed by
PetscObjectChangeTypeName), which would allow double-dispatch based on
table lookup.  (There are lots of variants of this idea, many C++
proposals force the linker to set up the table which is totally broken
with dynamic loading.)

Visitor isn't great as an extensible solution due to asymmetry and the
need to change the vtable of the other object.

A simple approach that is probably sufficient for VecLoadIntoVector
would be to move the pointer into viewer->ops because the viewer
typically just needs VecGetArray, and there is much more demand for new
viewer types than for new vector types.

Thoughts?

Jed



VecLoadIntoVector and double dispatch

2009-12-03 Thread Barry Smith

On Dec 3, 2009, at 3:57 PM, Jed Brown wrote:

> On Thu, 3 Dec 2009 15:09:03 -0600, Barry Smith   
> wrote:
>>But then your viewer must know about DM? That means that your
>> viewer knows about the internal structure of the vector?
>
> Actually, it doesn't use any internal knowledge about the vector,  
> but it
> does know about the DM (which has metadata like connectivity).

The vector has a pointer to the DM so the VecView() for that  
derived vector class has access to the DM information.  The same  
viewer object can be used with a bunch of different sized Vecs since  
it gets the DM information out of each vector. With your model when  
the DM info is attached to the viewer you need to pass in a different  
viewer for different vectors (of different sizes). This is just plan  
nuts.

>
>>I think this is very wrong. The viewer object is a lower level
>> object, it doesn't even know about linear algebra or vectors,  
>> nothing.
>> It should only expose a bunch of methods that the "higher level"
>> objects use. No viewer class show know about any Vec class or even  
>> the
>> existence of the Vec abstract class, but Vec classes can know about
>> the abstract Viewer class and even specific viewer classes.
>
> I'm struggling with this and the "loops" are troubling.
>
> So in a common scenario, we have a vector based on DMComposite which  
> is
> composed of, say, one DA and one unstructured DM (I call it FS).  It's
> the TS that will be making the call to write model state out at  
> special
> time steps and will need to read the state back later in the adjoint
> model.  The TS cannot know anything about the DM or implementation
> details of the Vec, therefore it is only allowed to call VecView,
> VecLoadIntoVector, and some viewer function to find time steps.
>
   Correct.

> So there needs to be some chunk of code that knows how to wire up this
> file with appropriate metadata, and some other chunks that can write
> Vecs and DMs wherever the Viewer tells it to.  The first clearly  
> belongs
> with the Viewer, the latter doesn't clearly (to me) belong on either
> side.  Since it only uses the VecGetArray level of information about  
> the
> vec, but uses an internal Viewer API, it seems more closely coupled to
> the viewer.

I say this is nonsense, it ain't more closely coupled to the viewer.

>  But somehow, calling VecView needs to be able to call the
> viewer with enough context for the viewer to wire up the metadata.   
> Note
> that the coordinates of the nodes in the DM are part of the vector  
> (it's
> an ALE formulation) so the metadata is nontrivial.

Each vector has a pointer to its DM (DMComposite etc) so its  
viewer can dump/read that info when needed. In fact, when writing a  
vector it could do something like

myvecview(Vec v,Viewer viewer)
{
 DM dm = getreferencetoDMfrom(v);
 DMView(dm,viewer);
 ... now dump my vector with writes or whatever(similar for  
reading in vectors with associated DMs).

in this way the viewing of your DM is nice encapsulated itself inside  
the DM object. You have to admit that it would be crazy to have the DM  
viewer method stored inside a viewer object which seems to be what  
your model is.

>
> In this case, I have a new viewer so having dispatch in the other
> objects is constraining because I would have to go modify every one  
> that
> I want my viewer to work with.  But if I was implementing a new Vec, I
> wouldn't want the viewer to be in charge of dispatch because then I'd
> have to go modify those viewers.
>
> I'd love to hear your suggestions.

You know my answer. The Viewers are abstractions of file IO, ;  
except instead of having only methods like read()/write() bytes it has  
somewhat "higher level" operations. Note: these methods can include  
graphics operations for example that are still essentially like  
writes(). YOU are making the viewers into these strange "all knowing"  
beasts which I think is wrong.

For a smart guy I'm surprised you've gone down this wrong track.

Barry


>
>
> Jed




VecLoadIntoVector and double dispatch

2009-12-03 Thread Barry Smith

On Dec 3, 2009, at 2:29 PM, Jed Brown wrote:

> Thanks for the explanation.
>
> On Thu, 3 Dec 2009 13:48:21 -0600, Barry Smith   
> wrote:
>>This is wrong. What about a binary viewer method for the PETSc Vec
>> class implemented in SAMRAI? This definitely cannot rely on
>> VecGetArray() and belongs with the Vec class not the viewer classes
>> (which know nothing about SAMRAI even existing.
>
> I don't know what is in SAMRAI's Vec, but how about my HDF5 viewer  
> that
> encodes information about the DM?  The Vec doesn't know anything about
> my viewer existing.

But then your viewer must know about DM? That means that your  
viewer knows about the internal structure of the vector?

I think this is very wrong. The viewer object is a lower level  
object, it doesn't even know about linear algebra or vectors, nothing.  
It should only expose a bunch of methods that the "higher level"  
objects use. No viewer class show know about any Vec class or even the  
existence of the Vec abstract class, but Vec classes can know about  
the abstract Viewer class and even specific viewer classes.


> Once we have more than one implementation of Viewer
> and more than one of Vec, no asymmetric solution can be perfect.  I  
> can
> live with static dispatch or other hacks for now.

I agree that no asymmetric solution is perfect, but I think it is  
very useful to have a hierarchy of classes that "know" about other  
classes; so the Vec class knows about Viewer classes but the Viewer  
classes do not know about Vec classes. Otherwise you get nasty "loops  
of knowledge" that are difficult to unwind.

Barry

>
> Jed




VecLoadIntoVector and double dispatch

2009-12-03 Thread Barry Smith

On Dec 3, 2009, at 12:54 PM, Jed Brown wrote:

> I'm sort of confused about vec->ops->loadintovectornative, this  
> seems to
> just be a way to let users provide *one* custom viewer on a particular
> vector without breaking PETSc's own viewers.  But it really doesn't
> provide a reasonably solution for an intermediate library, or user  
> code
> with multiple viewers.

Jed,

It is a hack to allow Vecs with particular layout properties (like  
arising from DA's) to overwrite the load/view operations that they  
inherit from the parent class, but still allow use of the parents  
method if desired. It is equivalent to the super operation in python  
or Java.


>
> Extensible double dispatch is not something C is heralded for, but  
> it's
> not impossible.  An easy-to-code approach is to just do it all with
> PetscObjectFunctionComposeDynamic.  The FList could get a bit long,  
> but
> I think you'd be hard-pressed to find a real code where dispatch time
> for VecLoadIntoVector and (a couple of other viewer-related functions)
> was an issue.  For best performance, PetscObject could acquire a field
> for "type index" or some such (e.g. managed by
> PetscObjectChangeTypeName), which would allow double-dispatch based on
> table lookup.  (There are lots of variants of this idea, many C++
> proposals force the linker to set up the table which is totally broken
> with dynamic loading.)

 This discussion about double dispatch is correct and PETSc does  
not do it properly. For VecView, for example, we dispatch off the  
vector with the function table but then dispatch off the viewer with  
the ugly, nonextendable if ()s.

However, I don't think this has much to do with your original point  
about loadintovectornative.  I view

PetscViewerPushtFormat(viewer,PETSC_VIEWER_NATIVE)
VecView(vec,viewer)

is equivalent to the python

super(self).view(viewer)  [may not be the exact right syntax]

or perhaps

super(VEC_MPI,self).view(viewer)

that is casting the vector up to its parent and then calling the  
viewer method of the parent.

Is this a great way to handle the "calling the method of the parent"?  
Well, probably not. If we wanted to handle it in general using a  
similar style we could have something like

PetscObjectPushUseSuper(vec)
VecView(vec,viewer)
PetscObjectPopUseSuper(vec)

then each derived class would just keep a copy of the previous  
function table (instead of overwriting the function pointers) and each  
Push() would cause it to use the next higher table to find the  
function pointer. So the information about which super of the Vec  
method we want should be stored with the Vec, NOT with the viewer. I  
just implemented as a VIEWER_FORMAT because we already had a viewer  
format mechanism and it was simple to do.  I did not have to introduce  
a hugh new infrastructure. Note that Babel uses the save the previous  
function table to handle its inheritance much as I describe (as Matt  
can attest it can all be done, but leads to relatively complicated  
code).

Note: I am not saying the current design is best or shouldn't be  
changed, it would just be complicated to change "properly" and is  
distinct from handling double dispatch properly.

>
> Visitor isn't great as an extensible solution due to asymmetry and the
> need to change the vtable of the other object.
>
> A simple approach that is probably sufficient for VecLoadIntoVector
> would be to move the pointer into viewer->ops because the viewer
> typically just needs VecGetArray, and there is much more demand for  
> new
> viewer types than for new vector types.

This is wrong. What about a binary viewer method for the PETSc Vec  
class implemented in SAMRAI? This definitely cannot rely on  
VecGetArray() and belongs with the Vec class not the viewer classes  
(which know nothing about SAMRAI even existing.

Barry


>
> Thoughts?
>
> Jed