dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Tue, Feb 24, 2009 at 10:47:33PM +0100, kent-and@xxxxxxxxx wrote:
> >
> >
> > Anders Logg wrote:
> >> On Tue, Feb 24, 2009 at 09:30:27PM +0000, A Navaei wrote:
> >>> 2009/2/24 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.
> >>> 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

Good point, I didn't think about refinement.

-- 
Anders

Attachment: signature.asc
Description: Digital signature