Anders Logg wrote: > On Wed, Apr 29, 2009 at 01:46:40PM +0200, Kent Andre wrote: >> On on., 2009-04-29 at 12:38 +0100, Garth N. Wells wrote: >>> Anders Logg wrote: >>>> On Wed, Apr 29, 2009 at 08:34:43AM +0200, Martin Sandve Alnæs wrote: >>>>> On Wed, Apr 29, 2009 at 4:50 AM, Shawn Walker <[email protected]> >>>>> wrote: >>>>>> 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 <[email protected]> >>>>>>>>> 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? :) >>>> It's better to have it in DOLFIN. It makes it easier to experiment (no >>>> need to change UFC). When features have stabilized, we move it to UFC. >>>> >>>> I agree having both ufc.h and UFC.h is confusing. How about this >>>> instead. In DOLFIN, we never include ufc.h, only UFC.h. > > What I meant was, we need to change this so we only include UFC.h in > places where we now include ufc.h. The inclusion of ufc.h will be > handled by UFC.h and it will pick the right one. > >>> We do include ufc.h in a number of places. This needs to be fixed so >>> that a ufc(e).h file can be placed in DOLFIN. >>> >>> Garth >>> >> What about generated code ? > > The experimental form compiler or the person hand-writing the > experimental code would need to handle that. >
There is come generated code in DOLFIN (in ale and mf) which complicates things. Garth _______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
