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. >>
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. Garth >> Where did you add this line? > > I change > > preprocessed_form = form > > to: > > preprocessed_form = form.form_data()._form > > Johan > >> -- >> 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@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