ffc team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

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?

--
Anders



> Johan
>
> > ?
> >
> > Garth
> >
> > >> Better would be
> > >>
> > >>     a.preprocess()
> > >>
> > >> or
> > >>
> > >>     a.form_data()
> > >
> > > As already mentioned in a previous email, I suggest we only call
> > > form_data(). This will return the form_data. The preprocessed form is
> > > attached to the form_data and this is what is passed to the code
> > > generator. I am pretty sure this is what was there from the beginning.
> > >
> > > It is confusing to call:
> > >   form = preprocess(form)
> > >
> > > as the preprocessed form was never ment to be doing anything but being
> > > passed to the code generator, AFAIK.
> > >
> > > Johan
> > >
> > >> Garth
> > >>
> > >>>> Garth
> > >>>>
> > >>>>>> Garth
> > >>>>>>
> > >>>>>>> def preprocess(form, object_names={}, common_cell=None):
> > >>>>>>>     ...
> > >>>>>>>
> > >>>>>>>     # Check that form is not already preprocessed
> > >>>>>>>
> > >>>>>>>     if form.form_data() is not None:
> > >>>>>>>         debug("Form is already preprocessed. Not updating form
> > >>>>>>>         data.") return form
> > >>>>>>>
> > >>>>>>>     ...
> > >>>>>>>
> > >>>>>>>     # Attach form data to form
> > >>>>>>>     form._form_data = form_data
> > >>>>>>>
> > >>>>>>>     # Attach preprocessed form to form data
> > >>>>>>>     form_data._form = form
> > >>>>>>>
> > >>>>>>> And when I look at the blamelist (bzr annotate), it looks like I
> > >>>>>>> added those lines, so I must have come to my senses and added it
> > >>>>>>> back at some point (way back). So in conclusion, calling
> > >>>>>>> preprocess() should not taking any time.
> > >>>>>>>
> > >>>>>>> What am I missing?
> > >>
> > >> _______________________________________________
> > >> Mailing list: https://launchpad.net/~ffc
> > >> Post to     : ffc@xxxxxxxxxxxxxxxxxxx
> > >> Unsubscribe : https://launchpad.net/~ffc
> > >> More help   : https://help.launchpad.net/ListHelp