dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Tue, Feb 24, 2009 at 08:49:27PM +0000, A Navaei wrote:
> 2009/2/24 Anders Logg <logg@xxxxxxxxx>:
> > On Tue, Feb 24, 2009 at 08:07:43PM +0000, A Navaei wrote:
> >> 2009/2/24 Johan Hake <hake@xxxxxxxxx>:
> >> > On Tuesday 24 February 2009 16:17:43 A Navaei wrote:
> >> >> 2009/2/24 Johan Hake <hake@xxxxxxxxx>:
> >> >> > 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

Attachment: signature.asc
Description: Digital signature