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. And at the top of that file, we put #ifdef UFC_EXPERIMENTAL #include "ufce.h" #else #include <ufc.h> #endif That should work. -- Anders
signature.asc
Description: Digital signature
_______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
