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@lists.launchpad.net > > >>>>>> Unsubscribe : https://launchpad.net/~ffc > > >>>>>> More help : https://help.launchpad.net/ListHelp > > > > _______________________________________________ > > Mailing list: https://launchpad.net/~ffc > > Post to : ffc@lists.launchpad.net > > Unsubscribe : https://launchpad.net/~ffc > > More help : https://help.launchpad.net/ListHelp > > _______________________________________________ > Mailing list: https://launchpad.net/~ffc > Post to : ffc@lists.launchpad.net > Unsubscribe : https://launchpad.net/~ffc > More help : https://help.launchpad.net/ListHelp _______________________________________________ Mailing list: https://launchpad.net/~ffc Post to : ffc@lists.launchpad.net Unsubscribe : https://launchpad.net/~ffc More help : https://help.launchpad.net/ListHelp