ufl team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxx
>>>>>> Unsubscribe : https://launchpad.net/~ffc
>>>>>> More help   : https://help.launchpad.net/ListHelp