Subject: Re: [FFC-dev] new mesh: quick but somewhat painful :)

From: Johan Jansson <[email-masked]>
Date: Wed, 1 Nov 2006 09:29:18 +0100

On Tue, Oct 31, 2006 at 05:16:33PM +0100, Dag Lindbo wrote:
> Hi all,
> 
> I'm seeing really great speedup due to the new mesh class in DOLFIN 0.6.3!
> Other reaction?
> 

Yes, I'm also seeing speedups, the old mesh had overhead which seems
to be gone now. I haven't done any real benchmarks yet though. The new
mesh is also a much better abstraction and I really like it.

> Well, I had to make changes in FFC in order to compile my solvers with
> 0.6.3. Let me share the problem (and a proposed fix).
> 
> As I understand it, there is a consolidation between DOLFIN and SWIG
> formats in DOLFIN 0.6.3/FFC 0.3.4 (Johan J: isn't this correct?).

Yes, this is correct, we had two different formats due to SWIG not
supporting nested classes. We've managed to come up with a workaround
though.

> The problem (that is new in 0.6.3) is that if one uses the same
> form-header in two classes (that are both used in a particular solver)
> then there will be an error at link-time due to multiple implementation of
> the form. Take this example:
>
> Class GeneralFunctions
>     *) Includes a form header, formX.h
>
> Class LevelSetSolver
>     *) Uses stuff from GenericFunctions
>
> Class FlowSolver
>      *) Uses stuff from GenericFunctions as well
>
> Class TwoPhaseFlowSolver
>      *) Uses FlowSolver and LevelSetSolver
>      *) Has main()
>
> All classes compile. This means that the code in the formX header is
> compiled into object code in multiple places. Then linker will fail due to
> redefinitions of the formX implementation.

Ok, the idea was that the new format should be semantically equivalent
to the old format. But it seems we overlooked one thing. The old
format looked like this:

class A
{
  void foo()
  {
  }
}

and the new format looks like this:

class A
{
  void foo();
}

void A::foo()
{
}

But apparently this is not equivalent, putting the implementation
inside the class definition implicitly makes the function inline, so
it should look like this:

class A
{
  void foo();
}

inline void A::foo()
{
}

> What I did to solve this was to change the output format of FFC so that I
> get form declarations in .h-files and form implementations in the
> corresponding .cpp-files. This way the form implementations are only
> compiled into object code once, so the linkage is fine.

Yes, this would be the cleanest solution. The question is if we want
to make such a structural change when we will change to a different
format soon anyway. It would break all the DOLFIN demos for example,
which assume only one source file.

> If anyone else is has plans to move to 0.6.3 and has forms that are used
> in multiple classes, then I think the best option is to do this change in
> FFC. Let me know, and I'll send a simple patch. Coming releases of
> FFC/DOLFIN will use a new format, and the linkage problem will be dealt
> with then (according to Anders Logg).
> 
> Cheers!
> /Dag Lindbo
> 

I CC:ed this to dolfin-dev and ffc-dev, I hope you don't mind.

  Johan