dolfin team mailing list archive

[Martin Sandve Alnæs <martinal@xxxxxxxxx>]

On 27 April 2011 16:00, Anders Logg <logg@xxxxxxxxx> wrote:

> 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.
>

I don't believe that it has actually worked without significant bugs as side
effects.
You've probably had lots of luck or subtle cache related bugs for a year or
so.

Martin