dolfin team mailing list archive

[Martin Sandve Alnæs <martinal@xxxxxxxxx>]

On Mon, Apr 27, 2009 at 5:49 PM, Anders Logg <logg@xxxxxxxxx> wrote:
> On Mon, Apr 27, 2009 at 04:42:59PM +0100, Garth N. Wells wrote:
>>
>>
>> Anders Logg wrote:
>> > On Mon, Apr 27, 2009 at 03:43:28PM +0200, 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.
>> >>
>> >> Martin
>> >
>> > I like having a separate header file for experimental stuff. Maybe it
>> > could be placed in DOLFIN? This will make it easier to experiment (no
>> > need for synchronization) and keep UFC constant.
>> >
>>
>> Having it in DOLFIN would be nice for those of us without write
>> permission to the ufc repository.
>>
>> Garth
>
> I'd be happy to give you write access if you want it (and Martin does
> not object).

No objection here.

> I thought the problem was I didn't have time to make the changes, but
> maybe the problem is no one else has access.
>
> Anyhow, any changes need to be discussed at some length. UFC is also
> fairly well documented (in contrast to other projects we know of) so
> any changes to the interface need to be documented in the manual.

Agreed. (When moved from the experimental interface to the official, anyway).

Martin