ffc team mailing list archive

[Johan Hake <johan.hake@xxxxxxxxx>]

On Tuesday April 26 2011 09:03:55 Anders Logg wrote:
> 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.

Just the first time. If a user need form_data it has to be generated anyhow, 
so why not on the fly? We do this other places.

Johan

> --
> 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