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. -- Anders
signature.asc
Description: Digital signature
_______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
