dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Wed, Dec 01, 2010 at 12:49:29PM +0000, Garth N. Wells wrote:
>
>
> On 01/12/10 12:45, Anders Logg wrote:
> >On Wed, Dec 01, 2010 at 11:26:42AM +0000, Garth N. Wells wrote:
> >>
> >>
> >>On 30/11/10 23:15, Anders Logg wrote:
> >>>On Tue, Nov 30, 2010 at 11:12:18PM +0000, Garth N. Wells wrote:
> >>>>
> >>>>
> >>>>On 30/11/10 23:09, Anders Logg wrote:
> >>>>>On Tue, Nov 30, 2010 at 10:48:09PM +0000, Garth N. Wells wrote:
> >>>>>>
> >>>>>>
> >>>>>>On 30/11/10 22:37, Johan Hake wrote:
> >>>>>>>On Tuesday November 30 2010 14:30:18 Garth N. Wells wrote:
> >>>>>>>>On 30/11/10 22:25, Anders Logg wrote:
> >>>>>>>>>On Tue, Nov 30, 2010 at 10:20:52PM +0000, Garth N. Wells wrote:
> >>>>>>>>>>We have a thread-safe problem in GenericFunction with the member object
> >>>>>>>>>>
> >>>>>>>>>>    mutable Data data;
> >>>>>>>>>>
> >>>>>>>>>>Any ideas on how to get rid of it? We really should pass data
> >>>>>>>>>>through the function interfaces, but we're constrained in this case
> >>>>>>>>>>by the UFC interface.
> >>>>>>>
> >>>>>>>This is ironical, as Data was introduced so we in the future (now present)
> >>>>>>>could be thread safe...
> >>>>>>
> >>>>>>Really? I remember Martin always warning against such a design
> >>>>>>because it's not thread-safe.
> >>>>>>
> >>>>>>>But I do not think GenericFunction was a ufc::function
> >>>>>>>at that time.
> >>>>>>>
> >>>>>>>Is it time to introduce the notion of thread in the ufc interface?
> >>>>>>>
> >>>>>>
> >>>>>>Not for this purpose. What would be helpful is a way to pass
> >>>>>>user-defined data through the UFC interface. Perhaps more
> >>>>>>importantly, we should avoid using 'mutable'. A 'const' object
> >>>>>>should in principle be thread-safe, but using mutable clouds this.
> >>>>>
> >>>>>Another solution (in addition to making the mutable data thread safe)
> >>>>>would be to extend the UFC interface with a void* optional argument
> >>>>>that can be passed to evaluate.
> >>>>>
> >>>>>Even if we add that, I suspect there will be a performance penalty if
> >>>>>we need to recreate the Data object each time in
> >>>>>restrict_as_ufc_function. In addition to convenience, Data is cached
> >>>>>between calls so it can be reused.
> >>>>>
> >>>>
> >>>>I'm trying the re-creation now. A good compiler might optimise away
> >>>>the overhead.
> >>>
> >>>Let's hope.
> >>>
> >>
> >>It seems that the only thing that we're missing relative to before
> >>is the local facet index. Everything else can be dealt with. Perhaps
> >>this could be added to the UFC interface, i.e.,
> >>
> >>   ufc::function::evaluate(double* values,
> >>                           const double* coordinates,
> >>                           const cell&  c)
> >>
> >>could become
> >>
> >>   ufc::function::evaluate(double* values,
> >>                           const double* coordinates,
> >>                           const cell&  c,
> >>                           int local_facet_index)
> >>
> >>?
> >
> >That sounds good. And -1 would signal that there the facet index is
> >not available.
> >
>
> Yes.
>
> We could have
>
>     ufc::function::evaluate(double* values,
>                             const double* coordinates,
>                             const cell&  c,
>                             int local_facet_index=-1)
>
> which won't break any code.

Would that help? Subclasses would need to subclass the correct
function. It would only make sure that we don't break C++ that uses
old generated UFC code (so old versions of DOLFIN would still work).

Instead we could overload with two different versions to ensure that
one can still use old generated code:

     virtual ufc::function::evaluate(double* values,
                                     const double* coordinates,
                                     const cell&  c)
     { evaluate(values, coordinates, c, -1); }

     virtual ufc::function::evaluate(double* values,
                                     const double* coordinates,
                                     const cell&  c,
                                     int local_facet_index);

The problem with that would be that none of these functions can be
made abstract (=0) since a user must be able to select which one to
overload.

--
Anders