dolfin team mailing list archive

[Shawn Walker <walker@xxxxxxxxxxxxxxx>]

On Thu, 30 Apr 2009, Anders Logg wrote:

> On Thu, Apr 30, 2009 at 08:54:23AM -0400, Shawn Walker wrote:
> > On Thu, 30 Apr 2009, Ola Skavhaug wrote:
> >
> > > Why not go for Martin's suggestion?
> > >
> > > Inheritance is a well known concept in C++, and it avoids using the
> > > evil macros that notoriously break compilation when people forget
> > > their ifndefs. Extending ufc with subclasss in a namespace, say eufc,
> > > dynamic casting in experimental assemblers when needed, and finally,
> > > moving functionality up the inheritance chain when settled seems to me
> > > a better solution than what is happening now?
> >
> > Why not do it?
> >
> > Reasons:
> >
> > 1. I'm not that adept at C++ to know how to do what you are saying.  Nor
> > do I forsee myself learning this in the near future. (i.e. like all this
> > dynamic casting stuff).
> >
> > 2. This `eufc' is only for experimentation and I would assume it will not
> > be a permanent piece of the code.
> >
> > 3. The original reason for doing this, was to basically just add some
> > freaking variables!
>
> I know, and that's exactly the point. Before we had UFC, the interface
> between FFC and DOLFIN changed all the time. Now we have two form
> compilers and other projects using UFC or considering using it. So we
> need to keep it stable and make sure we know what we want before
> making changes. Any change to ufc.h will require changes to DOLFIN, FFC,
> SyFi/SFC, and all code that has ever been generated for UFC 1.1.

ok, ok.

> > Why do we have to make it so complicated?
> > Furthermore, these additions (if made to the real ufc.h) would not have
> > changed anything.  The current modules now would just not use those new
> > data structs.  It would have been completely backwards compatible.  But I
> > understand you want to keep ufc and its doc's up to date.
> >
> > If someone wants to set this up, be my guest (see #1).  I would certainly
> > appreciate it.  I don't know how to do it.
>
> I talked to Martin and Ola today and they convinced me we should use
> inheritance, not macros (#ifdef).
>
> The reason is that with inheritance, we can freely extend the classes
> in ufc.h and the code (DOLFIN) will still work with the official UFC
> interface. With #ifdef, one would not be able to assemble forms
> compiled for the official UFC interface with DOLFIN when the
> UFC_EXPERIMENTAL flag is set.

ok.

> Here's a summary of what needs to be done:
>
> 1. Create a file named ufce.h and place it in dolfin/fem/ as part of
> DOLFIN.

This should be named ufcexp.h.  If someone is visually scanning the code, 
this can be missed.

> 2. This file defines subclasses of the UFC classes that need to be
> extended. It can look something like this:
>
> #include <ufc.h>
>
> namespace ufce

namespace ufcexp

> {
>
>  class cell : public ufc::cell
>  {
>  public:
>
>    /// Array of coordinates for higher order vertices of the cell
>    double** higher_order_coordinates;
>
>  };
>
> }
>
> 3. UFCCell needs to inherit from ufce::cell, not ufc::cell. That means
> it is still a ufc::cell since ufce::cell is a ufc::cell.

This seems reasonable...  again ufcexp::cell.  the other way can be easily 
overlooked.

> 4. In places where we assume that we have a ufce::cell and not a
> ufc::cell, we make a dynamic_cast and issue an error if the cast fails
> (which indicates that the form that comes in is not a ufce form but a
> ufc form). A dynamic_cast of a pointer can be done like this:
>
>  Bar* bar = dynamic_cast<Bar*>(foo);
>
> Here, foo is a pointer to a Foo and Bar is a subclass of Foo.
> If the cast fails, a zero pointer will be returned indicating that foo
> is not a Bar, only a Foo.

ok, so for example, say we are inside a tabulate_tensor routine.

when we read in `c.coordinates', everything is fine.  No problem.

Now, we want to read in the `higher_order_coordinates'.  So, we cannot do 
`c.higher_order_coordinates' because the routine only sees it as a 
ufc::cell (because we want to be backwards compatible).

So, we have to do this:

ufcexp::cell MYc = dynamic_cast<ufcexp::cell>(c);

Then we can read:

`MYc.higher_order_coordinates'.

Is this right?  If so, that doesn't sound too bad.

- Shawn