On 26/04/11 17:03, 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. >
What about what I suggested below? It requires explicit computation of the form data. I don't like having two form objects for what's basically the same object. This leads to circular dependencies, with each storing a reference to the other. Can't we having something like form._preprocessed_form = preprocess(form) and then not have the preprocessed form storing a reference to the original form? Garth > -- > 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 _______________________________________________ Mailing list: https://launchpad.net/~ffc Post to : ffc@lists.launchpad.net Unsubscribe : https://launchpad.net/~ffc More help : https://help.launchpad.net/ListHelp