ffc team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

On 26/04/11 17:16, Anders Logg wrote:
> On Tue, Apr 26, 2011 at 06:12:26PM +0200, Martin Sandve Alnæs wrote:
>> On 26 April 2011 18:10, Johan Hake <johan.hake@xxxxxxxxx> wrote:
>>
>>     On Tuesday April 26 2011 09:01:35 Anders Logg wrote:
>>     > On Tue, Apr 26, 2011 at 08:44:24AM -0700, Johan Hake wrote:
>>     > > On Tuesday April 26 2011 08:42:32 Anders Logg wrote:
>>     > > > On Tue, Apr 26, 2011 at 08:39:30AM -0700, Johan Hake wrote:
>>     > > > > On Tuesday April 26 2011 08:33:11 Garth N. Wells wrote:
>>     > > > > > On 26/04/11 16:31, Johan Hake wrote:
>>     > > > > > > On Tuesday April 26 2011 08:16:29 Garth N. Wells wrote:
>>     > > > > > >> On 26/04/11 16:07, Anders Logg wrote:
>>     > > > > > >>> On Tue, Apr 26, 2011 at 03:59:52PM +0100, Garth N. Wells
>>     wrote:
>>     > > > > > >>>> On 26/04/11 15:55, Anders Logg wrote:
>>     > > > > > >>>>> On Tue, Apr 26, 2011 at 03:45:22PM +0100, Garth N. Wells
>>     wrote:
>>     > > > > > >>>>>> On 26/04/11 13:51, Anders Logg wrote:
>>     > > > > > >>>>>>> On Tue, Apr 26, 2011 at 02:00:50PM +0200, Anders Logg
>>     wrote:
>>     > > > > > >>>>>>>> It feels good that you trust me enough to handle it. ;-)
>>     > > > > > >>>>>>>>
>>     > > > > > >>>>>>>> Will add it sometime this afternoon and then we can
>>     > > > > > >>>>>>>> revisit the JIT compiler caching.
>>     > > > > > >>>>>>>
>>     > > > > > >>>>>>> I'm getting confused here... Looking at preprocess.py in
>>     > > > > > >>>>>>> UFL, I see
>>     > > > > > >
>>     > > > > > > this:
>>     > > > > > >>>>>> It is confusing. Does the function 'preprocess' do
>>     anything
>>     > > > > > >>>>>> that the old FormData class didn't? It would be easier to
>>     > > > > > >>>>>> follow if Form just had a member function form_data() that
>>     > > > > > >>>>>> computes and stores data (like it used to), or if Form had
>>     > > > > > >>>>>> a 'preprocess' function. Having the function preprocess
>>     > > > > > >>>>>> return a new form is really confusing.
>>     > > > > > >>>>>
>>     > > > > > >>>>> I don't find that particularly confusing. It's the same as
>>     > > > > > >>>>>
>>     > > > > > >>>>>   refined_mesh = refine(mesh)
>>     > > > > > >>>>
>>     > > > > > >>>> Which is the whole problem. By creating a new object,
>>     FormData
>>     > > > > > >>>> is thrown away. The preprocessing should just compute some
>>     > > > > > >>>> more data, just like we *don't* do
>>     > > > > > >>>>
>>     > > > > > >>>>   initialised_mesh = mesh.init(0)
>>     > > > > > >>>>
>>     > > > > > >>>> What was wrong with Martin's original design that
>>     necessitated
>>     > > > > > >>>> the change?
>>     > > > > > >>>
>>     > > > > > >>> As I explained, I thought it was better to have an explicit
>>     > > > > > >>> call to preprocess since that makes it clear that one makes a
>>     > > > > > >>> call to a function which may take some time to execute
>>     > > > > > >>> (instead of just calling a member function which seems to
>>     just
>>     > > > > > >>> return some data).
>>     > > > > > >>>
>>     > > > > > >>> But as I say above: I added the caching back at some point
>>     > > > > > >>> (maybe even the day after I removed it 2 years ago) so we
>>     > > > > > >>> don't need to discuss why I removed it (as I realized myself
>>     I
>>     > > > > > >>> shouldn't have removed it and added it back a long time ago).
>>     > > > > > >>>
>>     > > > > > >>> What has me confused now is that the caching seems to be in
>>     > > > > > >>> place but we still need the extra caching in FFC/DOLFIN and I
>>     > > > > > >>> don't see why.
>>     > > > > > >>
>>     > > > > > >> Because preprocess returns a new form, e.g. define a form
>>     > > > > > >>
>>     > > > > > >>   a = u*v*dx
>>     > > > > > >>   jit(a)
>>     > > > > > >>
>>     > > > > > >> Inside jit,
>>     > > > > > >>
>>     > > > > > >>    a.form_data() is None:
>>     > > > > > >>        b = preprocess(a) # b now has data attached, but a
>>     > > > > > >>        doesn't
>>     > > > > > >>
>>     > > > > > >>    else:
>>     > > > > > >>        b = a
>>     > > > > > >>
>>     > > > > > >> Now 'b' has been preprocessed, and has form data attached, but
>>     > > > > > >> 'a' doesn't. Calling 'jit(a)' again, the code will never enter
>>     > > > > > >> the 'else' part of the clause because 'a' never gets any form
>>     > > > > > >> data. Johan has added some code FFC that attaches the form
>>     data
>>     > > > > > >> of 'b' to 'a', but it is a bit clumsy.
>>     > > > > > >
>>     > > > > > > No, it was already attached. I just made ffc use it.
>>     > > > > >
>>     > > > > > Didn't you add the line
>>     > > > > >
>>     > > > > >     form._form_data = preprocessed_form.form_data()
>>     > > > >
>>     > > > > No, I added:
>>     > > > >   preprocessed_form = form.form_data()._form
>>     > > > >
>>     > > > > I think the thing here is that form_data has always had a
>>     > > > > preprocessed form. Someone (lets not point fingers!) thought that
>>     > > > > was too much magic and added an
>>     > > > >
>>     > > > > explicit need to call:
>>     > > > >   form = preprocess(form)
>>     > > > >
>>     > > > > in jit_compiler(). This made the design more complicated and also
>>     > > > > introduced a cirucular dependency, as the return preprocessed form
>>     > > > > need to know of its form_data, but the form_data already had a
>>     > > > > reference to the preprocessed form. The latter is what I used in
>>     the
>>     > > > > one line I altered.
>>     > > >
>>     > > > No, it made the design cleaner since it makes clear something needs
>>     to
>>     > > > happen to get the metadata: a call to preprocess.
>>     > > >
>>     > > > Where did you add this line?
>>     > >
>>     > > I change
>>     > >
>>     > >   preprocessed_form = form
>>     > >
>>     > > to:
>>     > >   preprocessed_form = form.form_data()._form
>>     >
>>     > Yes, but where?
>>     >
>>     > I've fixed the bug now in preprocess.py (attaching to both forms). Does
>>     > that help?
>>
>>     In ffc.jit_form.
>>
>>     Your fix wont fix the circular dependency.
>>
>>     We also need to remove form_data from the preprocessed form.
>>
>>
>> Or just use a weakref.
>>
>>
>>     This means that
>>     we need to return form_data from preprocess and maybe change its name to
>>     compute_form_data.
>>
>>     Johan
> 
> Why is the circular dependency a problem? Anyway, I'm thinking now the
> cleanest design would be
> 
>   form.compute_form_data()
>   form_data = form.form_data()
>   preprocessed_form = form_data.preprocessed_form
> 
> Here, the preprocessed_form would not store form_data.
> 

Agree.

Garth

> --
> Anders