On Thu, Apr 30, 2009 at 02:02:31PM -0400, Shawn Walker wrote: > > 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.
Good point.
>> 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.
Sure.
>> 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.
Almost:
const ufcexp::cell* cexp = dynamic_cast<const ufcexp::cell*>(c);
dolfin_assert(cexp);
cexp->higher_order_coordinates[...]
It needs to be a pointer and it also needs to be const.
You might also get around with
const ufcexp::cell& cexp = *dynamic_cast<const ufcexp::cell*>(c);
cexp.higher_order_coordinates[...]
This should throw an exception (bad_cast) if it does not work.
But perhaps the best would be
const ufcexp::cell* cexp = dynamic_cast<const ufcexp::cell*>(c);
if (!cexp)
error("Informative error message.");
--
Anders
signature.asc
Description: Digital signature
_______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
