VecLoadIntoVector and double dispatch
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
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
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
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
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
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
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
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
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
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
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
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