ffc team mailing list archive

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

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