← Back to team overview

dolfin team mailing list archive

Re: DOLFIN-based variational image processing now available

 

>
>
> 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



Follow ups

References