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