dolfin team mailing list archive

[Shawn Walker <walker@xxxxxxxxxxxxxxx>]

On Tue, 28 Apr 2009, Anders Logg wrote:

> On Mon, Apr 27, 2009 at 11:39:08PM -0400, Shawn Walker wrote:
> > On Mon, 27 Apr 2009, Martin Sandve Aln?s wrote:
> >
> > > On Mon, Apr 27, 2009 at 3:32 PM, Garth N. Wells <gnw20@xxxxxxxxx> wrote:
> > > > Anders Logg wrote:
> > > > > On Mon, Apr 27, 2009 at 09:21:58AM -0400, Shawn Walker wrote:
> > > > > > On Mon, 27 Apr 2009, Anders Logg wrote:
> > > > > >
> > > > > > > On Mon, Apr 27, 2009 at 11:26:13AM +0200, Kent Andre wrote:
> > > > > > > > On l?., 2009-04-25 at 00:14 +0200, Anders Logg wrote:
> > > > > > > > > On Wed, Apr 22, 2009 at 05:28:30PM -0400, Shawn Walker wrote:
> > > > > > > > > > Here is the changeset that adds a `higher_order_coordinates' variable for
> > > > > > > > > > storing higher order mesh data.  This is a very minor change so please
> > > > > > > > > > push this.
> > > > > > > > > >
> > > > > > > > > > A changeset for DOLFIN is coming immediately after this.
> > > > > > > > > >
> > > > > > > > > > - Shawn
> > > > > > > > > I'm not sure what to do about this. It's problematic to add
> > > > > > > > > experimental work to UFC since it must be stable. In particular, any
> > > > > > > > > small change to ufc.h means that all forms must be recompiled
> > > > > > > > > everywhere for everyone.
> > > > > > > > >
> > > > > > > > > So before we make a change to UFC, we need to know exactly what we
> > > > > > > > > need. Which also means I can't import your DOLFIN patch since it
> > > > > > > > > depends on the UFC patch.
> > > > > > > > >
> > > > > > > > > I see you've added
> > > > > > > > >
> > > > > > > > >     double** higher_order_coordinates;
> > > > > > > > >
> > > > > > > > > to ufc::cell. This is analogous to what is now implemented in
> > > > > > > > > MeshGeometry and the mesh XML format so I think it's good.
> > > > > > > > >
> > > > > > > > > The question is what other information we need. As it works now (for
> > > > > > > > > the standard ufc::cell), UFC code generated by a form compiler knows
> > > > > > > > > what to expect from for a ufc::cell argument. If higher order mappings
> > > > > > > > > should work the same way, then the generated code and thus the form
> > > > > > > > > compilers need to know which mapping should be used and also the
> > > > > > > > > length of higher_order_coordinates. Is this what you were thinking?
> > > > > > > > >
> > > > > > > > > Before we do much more about it, more people need to weigh in on it as
> > > > > > > > > it affects DOLFIN, UFC, SyFi and FFC.
> > > > > > > >
> > > > > > > > But is there any other way around this. It would be nice with higher
> > > > > > > > order meshes and UFC should not stop this.
> > > > > > > >
> > > > > > > > An alternative to changing the cell class would be to make a subclass
> > > > > > > > of cell. Would this work ?
> > > > > > > How about just using the current ufc::cell data structure as it is but
> > > > > > > let coordinates hold all the coordinates?
> > > > > > >
> > > > > > > This could also be the final solution. Then everything that's needed
> > > > > > > is an extra argument to tabulate_tensor that tells the generated code
> > > > > > > whether the cell is affinely mapped or not. The flag could simply be
> > > > > > > an integer: 1 means affine, 2 means quadratic etc.
> > > > > > But you still need to modify the ufc::cell code, I think.  There is also
> > > > > > an implicit assumption that the higher order coordinates should contain
> > > > > > the standard mesh vertex coordinates.  Of course, this is true for most
> > > > > > practical cases.  But for more fancy mappings, maybe this is not the
> > > > > > case.
> > > > >
> > > > > It seems to me that a reasonable assumption would be to limit the
> > > > > cases to P1, P2, P3, etc, that is, mappings that can be written down
> > > > > using standard Lagrange bases so then the vertices will always be
> > > > > included. They would also be first in the list meaning that the code
> > > > > would actually work (but might not give accurate results) even if it
> > > > > were generated for affine mappings.
> > > > >
> > > > > > Also, in the ufc::cell code, you currently read in the cell coordinates
> > > > > > using info in MeshTopology.  However, the higher order coordinate info
> > > > > > resides in MeshGeometry (which is where it belongs).  So you would still
> > > > > > need to modify ufc.h.   Remember, there is higher order cell data that is
> > > > > > contained in MeshGeometry.
> > > > >
> > > > > Where is MeshTopology used for this? I looked in UFCCell.h which is
> > > > > where the coordinates are copied to ufc::cell and there MeshGeometry
> > > > > is used.
> > > > >
> > > > > > Is it really that hard to change ufc.h?  Other things have to be
> > > > > > recompiled, but isn't that automatic?
> > > > >
> > > > > Yes, it's easy to change, but a main point with UFC is that we
> > > > > shouldn't change it.
> > > >
> > > > UFC will need to be extended as time goes on, but it is hard to know
> > > > from the outset how it should be done. What about using some IFDEF's or
> > > > non-pure virtual functions in the development version to allow
> > > > experimentation? These can then either be removed or added to UFC at
> > > > release time.
> > > >
> > > > Garth
> > >
> > > Or subclasses with non-pure virtual functions:
> > >
> > > class experimental_cell_integral: public ufc::cell_integral
> > > {
> > >  void foo() const { throw ...("Experimental feature not implemented."); }
> > > };
> > >
> > > or
> > >
> > > namespace eufc {
> > >  class cell_integral: public ufc::cell_integral
> > >  {
> > >    void foo() const { throw ...("Experimental feature not implemented."); }
> > >  };
> > > }
> > >
> > > We can define these in "experimental_ufc.h" or "eufc.h" to keep the
> > > official header file constant.
> > >
> > > Then the DOLFIN code that uses experimental features must be clearly marked:
> > >
> > >  ufc::cell_integral *itg = form.create_cell_integral(0)
> > >  eufc::cell_integral *eitg = dynamic_cast<eufc::cell_integral>(itg);
> > >
> > > and can then use "if(eitg)" to select between experimental and
> > > non-experimental code.
> >
> > In a similar vein, could we just have another file named eufc.h, and put
> > an IFNDEF somewhere that would use eufc.h instead of ufc.h?  That way I
> > could modify eufc.h all I want, and people don't have to use it.  But I'm
> > not sure how to do this.  Ideally, this would be an option for scons like
> > enableExpUFC=true.  Or is it only necessary to include eufc.h in files
> > that FFC generates?  We just need something for testing.
> >
> > Obviously, I cannot just modify UFCCell.h.  I tried that, but FFC cannot
> > access variables declared in the sub-class (woops...  :(  ).
>
> One option could be to create a file named ufc.h and put it in
>
>  dolfin/fem/ufc.h
>
> and change all #include <ufc.h> to #include "ufc.h".
>
> Then the DOLFIN version of ufc.h will include either the installed
> ufc.h or another file named ufce.h which is placed in
>
>  dolfin/fem/ufce.h
>
> That file contains data structures named the same way as in the
> official ufc.h but with modifications (so the rest of the code won't
> need to be changed much).
>
> Then in the DOLFIN ufc.h control file we place an #ifdef for whether
> to include the official ufc.h or the experimental one:
>
> #ifdef UFC_EXPERIMENTAL
> #include "ufce.h"
> #else
> #include <ufc.h>
> #endif
>
> You can set the flag by adding
>
>  customCxxFlags="-DUFC_EXPERIMENTAL"
>
> to scons.

Wouldn't it make more sense to have this exp_ufc.h be a part of ufc?  That 
way when you install ufc, there can be an `enableExpUFC' option.  In this 
case, scons will copy exp_ufc.h to the build directory and rename it to 
ufc.h.  The exp_ufc.h file will basically be identical to ufc.h, so it 
makes sense to keep it together.  This will also put in a convenient 
`buffer' for experimenting with ufc, and allow for easy moving over of 
additions to ufc.h.  Could someone please put this in?  Please?  :)

- Shawn