← Back to team overview

ufl team mailing list archive

Re: [Dolfin] [Branch ~ufl-core/ufl/main] Rev 1014: Add warnings to set_foo functions in finiteelement.py,

 

On Wed, Apr 27, 2011 at 03:57:13PM +0200, Martin Sandve Alnæs wrote:
> On 27 April 2011 15:49, Anders Logg <logg@xxxxxxxxx> wrote:
>
>     On Wed, Apr 27, 2011 at 02:39:17PM +0100, Garth N. Wells wrote:
>     >
>     >
>     > On 27/04/11 14:34, Anders Logg wrote:
>     > > On Wed, Apr 27, 2011 at 03:24:05PM +0200, Martin Sandve Aln s wrote:
>     > >> On 27 April 2011 12:38, Garth N. Wells <gnw20@xxxxxxxxx> wrote:
>     > >>
>     > >>
>     > >>
>     > >>     On 27/04/11 11:08, Kristian  lgaard wrote:
>     > >>     > On 27 April 2011 11:52, Anders Logg <logg@xxxxxxxxx> wrote:
>     > >>     >> On Wed, Apr 27, 2011 at 10:40:50AM +0100, Garth N. Wells wrote:
>     > >>     >>> We need a quick discussion round to resolve this issue -
>     DOLFIN is now
>     > >>     >>> broken. I guess we need the form compiler to decide on the
>     cell and
>     > >>     >>> element type, and then have UFL return a new form.
>     > >>     >>
>     > >>     >> Yes, Martin mentioned at some point that it would be easy to
>     add such
>     > >>     >> a function to UFL (that takes a form and replacement elements
>     and
>     > >>     >> returns a new form).
>     > >>     >>
>     > >>     >> Martin, could you add such a function?
>     > >>
>     > >>
>     > >> I started some time before easter, but as Garth says...
>     > >>
>     > >>
>     > >>     > I think ufl.algorithm.transformations.replace would work if
>     > >>     > FiniteElementBase derived from 'Terminal'.
>     > >>     > Currently, it derives from 'object', what is the reason for
>     that?
>     > >>     > Anyway, it should be simple enough to add something equivalent
>     for
>     > >>     elements.
>     > >>     >
>     > >>
>     > >>     I was thinking that we only need to replace Coefficients, e.g. if
>     a
>     > >>     Coefficient is defined using an 'incomplete element', FFC can
>     create an
>     > >>     element, a Coefficient and the perform the replacement. Looks like
>     UFL
>     > >>     can do this already.
>     > >>
>     > >>     The problem from the DOLFIN side is that the new UFL Coefficient
>     will be
>     > >>     different from the DOLFIN/UFL Coefficient, and would be a
>     > >>     ufl.Coefficient and not a dolfin.Coefficient.
>     > >>
>     > >>
>     > >> ... this may be a problem. Since I don't really know the motivation
>     for this
>     > >> feature request, I can't solve this problem for you.
>     > >
>     > > The motivation is that we want to accomplish what we now accomplish
>     > > with the set_cell/degree functions that you have decided to remove.
>     > >
>     > > This allows us to write f = Expression("sin(x[0])") and let the form
>     > > compiler choose a suitable approximation for the expression.
>     > >
>     >
>     > Just to keep the caching discussion alive ;), perhaps a DOLFIN
>     > Expression (which is a subclass of ufl.Coefficient) with an incomplete
>     > function space could have a cache ufl.Expressions that have completely
>     > defined elements, but which share the eval() function of the original. I
>     > don't know on a technical level how to make this work.
>     >
>     > The JIT compiler would need to return a map of replaced -> new
>     coefficients.
>
>     I don't really understand why the set_degree/cell functions are so
>     bad. I understand that a form should be immutable, but if the cell and
>     degree are set to "?", we're not really changing anything.
>
>
> 1) You don't know who else may have references to those objects. All bets are
> off.
>
> 2) You change the hash of the objects. That messes up dicts and sets that have
> them as keys. Severly.
>
> 3) The hack to update the repr of the objects isn't working, because
> expressions that reference them may have their own cached repr strings which
> won't get updated. Even if that wasn't a problem, dicts and sets would be
> messed up by the changed hashes anyway.
>
>
>     Perhaps it would be enough to insert a check in the set_cell/degree
>     functions that they can only be called on objects where the cell/degree
>     has not been set?
>
>
> Wouldn't change any of the above reasons.

I see your points but this has worked fine for a year or so. It's an
important feature so it's essential we find an alternative way to
handle unspecified elements.

--
Anders



Follow ups

References