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. > > 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 ? Kent _______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
