On 26 April 2011 18:07, Johan Hake <johan.h...@gmail.com> wrote:
> On Tuesday April 26 2011 09:03:55 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.
>
> Just the first time. If a user need form_data it has to be generated
> anyhow,
> so why not on the fly? We do this other places.
>
> Johan
Why not just rename it to compute_form_data() then?
Martin
> --
> > 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
