> > > Anders Logg wrote: >> On Tue, Feb 24, 2009 at 09:30:27PM +0000, A Navaei wrote: >>> 2009/2/24 Anders Logg <[email protected]>: >>>> On Tue, Feb 24, 2009 at 08:49:27PM +0000, A Navaei wrote: >>>>> 2009/2/24 Anders Logg <[email protected]>: >>>>>> On Tue, Feb 24, 2009 at 08:07:43PM +0000, A Navaei wrote: >>>>>>> 2009/2/24 Johan Hake <[email protected]>: >>>>>>>> On Tuesday 24 February 2009 16:17:43 A Navaei wrote: >>>>>>>>> 2009/2/24 Johan Hake <[email protected]>: >>>>>>>>>> On Tuesday 24 February 2009 01:08:11 A Navaei wrote: >>>>>>>>>>> Finally after all those long discussions on the best way of >>>>>>>>>>> architecturing variational image processing problems based on >>>>>>>>>>> dolfin, >>>>>>>>>>> a minimal demo showing how to solve a classical motion >>>>>>>>>>> estimation PDE >>>>>>>>>>> is available now -- thanks for all the support. Detailed >>>>>>>>>>> explanation >>>>>>>>>>> is given here: >>>>>>>>>>> >>>>>>>>>>> http://code.google.com/p/debiosee/wiki/DemosOptiocFlowHornSchunck >>>>>>>>>>> >>>>>>>>>>> Currently, the c++ implementation is done and the python >>>>>>>>>>> wrapper is on >>>>>>>>>>> the way. >>>>>>>>>> Cool! >>>>>>>>>> >>>>>>>>>>> I have tried to perform sub-classing as much as possible and >>>>>>>>>>> leave the >>>>>>>>>>> rest to be implemented outside of the main classes. While this >>>>>>>>>>> works, >>>>>>>>>>> there is a tiny problem which I'm not quite happy about. Here >>>>>>>>>>> is >>>>>>>>>>> what's happening: >>>>>>>>>>> >>>>>>>>>>> [ITK-backend] >>>>>>>>>>> >>>>>>>>>>> v >>>>>>>>>>> [GenericImage]-------\ >>>>>>>>>>> >>>>>>>>>>> v | >>>>>>>>>>> [ImageToMesh] | >>>>>>>>>>> >>>>>>>>>>> v | >>>>>>>>>>> [FunctionSpace] | >>>>>>>>>>> >>>>>>>>>>> v | >>>>>>>>>>> [ImageFunction]<-----/ >>>>>>>>>>> >>>>>>>>>>> If you still cannot see what's happening, get a monospace font >>>>>>>>>>> :) What >>>>>>>>>>> I don't like is the right branch connecting GenericImage to >>>>>>>>>>> ImageFunction. There should be a way of making sure that the >>>>>>>>>>> two >>>>>>>>>>> branches are initiated from the same image source, or this >>>>>>>>>>> could be a >>>>>>>>>>> source of error. Simply encapsulating this in a class takes >>>>>>>>>>> away the >>>>>>>>>>> freedom of defining a general problem away from the user. >>>>>>>>>> Just a thought: >>>>>>>>>> >>>>>>>>>> Would it help to let GenericImage also be a dolfin::Mesh? Then >>>>>>>>>> in >>>>>>>>>> ITKImage you copy paste the algorithm for creating a >>>>>>>>>> UnitCube/UnitSquare/Rectange/Box (the algorithms are actually >>>>>>>>>> not very >>>>>>>>>> large). >>>>>>>>> I understand that implemented algorithms are short, but maybe >>>>>>>>> it's not >>>>>>>>> a good practice to copy/paste the code (even if it's a short >>>>>>>>> code). We >>>>>>>>> should think of a more creative way of doing this. >>>>>>>> Sure ;) >>>>>>>> >>>>>>>> One alternative could be to inherit UnitSquare directly but then >>>>>>>> we need to >>>>>>>> put the creation algorithm in, e.g., an init function (protected >>>>>>>> such ;) ). >>>>>>>> This will then be called in the constructor, and we also need an >>>>>>>> empty >>>>>>>> constructor to be called by the inherited mesh, which may or may >>>>>>>> not make any >>>>>>>> sense at all. >>>>>>> This looks like something feasible to me. What about having a >>>>>>> virtual >>>>>>> Mesh::Init() called in Mesh() so that the subclasses just have to >>>>>>> override Init()? Then in the ImageMesh case, the parent Init() >>>>>>> should >>>>>>> be also called and the extra branch in the diagram will disappear. >>>>>> It doesn't seem to be a good idea to make the construction of the >>>>>> mesh >>>>>> optional in subclasses of UnitSquare. Generally, we try to avoid >>>>>> init() functions (as we've had plenty of them before and it >>>>>> complicates the design). >>>>>> >>>>>> It would be better to do something like this: >>>>>> >>>>>> class ImageMesh : public Mesh >>>>>> { >>>>>> public: >>>>>> >>>>>> ImageMesh(const Image& image) : Mesh() >>>>>> { >>>>>> // Compute nx, ny etc >>>>>> >>>>>> // Create unit square mesh and copy it >>>>>> UnitSquare mesh(nx, ny); >>>>>> *this = mesh; >>>>>> } >>>>>> >>>>>> }; >>>>> I did try this before creating ImageToMesh. I guess it needs the >>>>> right >>>>> copy constructor as I get this error: >>>>> >>>>> error: no match for ?operator=? in ?*(debiosee::ImageMesh*)this = >>>>> mesh? >>>>> >>>>> then, creating the copy constructor seems unnecessary since we have >>>>> to >>>>> copy everything that the subclass owns. I also tried downcasting >>>>> using >>>>> dynamic_cast, but it didn't work. >>>> The following should work: >>>> >>>> static_cast<Mesh&>(*this) = mesh; >>>> >>>> Look at the misc sandbox. >>> Thanks, I justed tried it in debiosee and it works. >>> >>> Regarding storing the cell number infomation in ordered meshes, is >>> this going to be added in dolfin? >>> >>> -Ali >> >> Do you mean nx, ny etc? >> >> Yes, that can be added. It also needs to be added for other built-in >> meshes. Send a patch if you want it fast. >> >> I'd suggest something like >> >> class UnitSquare : public Mesh >> { >> public: >> >> enum Type {right, left, crisscross}; >> >> /// Create mesh of unit square >> UnitSquare(uint nx, uint ny, Type type = right); >> >> /// Return number of cells in x-direction >> uint nx() const; >> >> /// Return number of cells in x-direction >> uint ny() const; >> >> private: >> >> uint _nx; >> uint _ny; >> >> }; >> >> etc. >> > > Not sure that it's so simple. Remember that the mesh can be refined, in > which case the dimensions will change, and in the case of local > refinement dimensions won't make sense. > > Garth >
Agree. Dolfin is all about unstructured meshes as it is now. I think you in general need to keep your additional knowledge about the structure in separate classes, at least for now. Kent _______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
