dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

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

Attachment: signature.asc
Description: Digital signature