| Thread Previous • Date Previous • Date Next • Thread Next |
On Thu, 30 Apr 2009, Ola Skavhaug wrote:
Why not go for Martin's suggestion? Inheritance is a well known concept in C++, and it avoids using the evil macros that notoriously break compilation when people forget their ifndefs. Extending ufc with subclasss in a namespace, say eufc, dynamic casting in experimental assemblers when needed, and finally, moving functionality up the inheritance chain when settled seems to me a better solution than what is happening now?
Why not do it? Reasons:1. I'm not that adept at C++ to know how to do what you are saying. Nor do I forsee myself learning this in the near future. (i.e. like all this dynamic casting stuff).
2. This `eufc' is only for experimentation and I would assume it will not be a permanent piece of the code.
3. The original reason for doing this, was to basically just add some freaking variables! Why do we have to make it so complicated? Furthermore, these additions (if made to the real ufc.h) would not have changed anything. The current modules now would just not use those new data structs. It would have been completely backwards compatible. But I understand you want to keep ufc and its doc's up to date.
If someone wants to set this up, be my guest (see #1). I would certainly appreciate it. I don't know how to do it.
- Shawn
On Wed, Apr 29, 2009 at 1:36 PM, Anders Logg <logg@xxxxxxxxx> 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 <walker@xxxxxxxxxxxxxxx> 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 <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. - ShawnI'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. GarthOr 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 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) iEYEARECAAYFAkn4O6IACgkQTuwUCDsYZdG3bQCglt8WQtlFN3U/s73vkEr+rS0E lYIAn0tSAnDOgHgyEfnE48ZMFJecjfB8 =p76f -----END PGP SIGNATURE----- _______________________________________________ UFC-dev mailing list UFC-dev@xxxxxxxxxx http://fenics.org/mailman/listinfo/ufc-dev-- Ola Skavhaug _______________________________________________ UFC-dev mailing list UFC-dev@xxxxxxxxxx http://fenics.org/mailman/listinfo/ufc-dev
| Thread Previous • Date Previous • Date Next • Thread Next |
