ffc team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Tue, Apr 26, 2011 at 09:01:37AM -0700, Johan Hake wrote:
> On Tuesday April 26 2011 08:48:33 Garth N. Wells wrote:
> > On 26/04/11 16:44, 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.
>
> Why is:
>
>   form_data = form.form_data()
>   preprocessed_form = form_data._form
>
> so bad?

Since it makes it look like form_data() just returns existing data
when it actually leads to an expensive computation.

--
Anders


> > How about something like
> >
> >   a.compute_form_data()
> >
> > to compute the data, and
> >
> >   data = a.form_data()
> >
> > to get the FormData. This is like Martin's orginal design, except
> > form_data() returns None if the data hasn't been computed.
>
> I think this adds more to the form than is nessesary.
>
> Johan
>
> > Garth
> >
> > >> Where did you add this line?
> > >
> > > I change
> > >
> > >   preprocessed_form = form
> > >
> > > to:
> > >   preprocessed_form = form.form_data()._form
> > >
> > > Johan
> > >
> > >>> 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
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~ffc
> > Post to     : ffc@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~ffc
> > More help   : https://help.launchpad.net/ListHelp
>
> _______________________________________________
> Mailing list: https://launchpad.net/~ffc
> Post to     : ffc@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~ffc
> More help   : https://help.launchpad.net/ListHelp