dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Wed, Dec 01, 2010 at 01:18:47PM +0000, Garth N. Wells wrote:
>
>
> On 01/12/10 13:02, Anders Logg wrote:
> >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.
> >
>
> I'm having doubts about this approach - the UFC functions restrict
> on cells, so the way we do it now is a bit of a hack (using the
> local facet).

Yes, it's a bit of a hack, but it would be less of a hack if we added
local_facet_index to the UFC interface and described its role.

--
Anders