Feel free to do it :) Martin
On 26 April 2011 13:55, Anders Logg <l...@simula.no> wrote: > So will you add it back? I can do it since it's my fault removing it, > but I assume you want to handle it yourself. ;-) > > -- > Anders > > > On Tue, Apr 26, 2011 at 01:46:59PM +0200, Martin Sandve Alnæs wrote: > > Caching is ok precicely _because_ the form is immutable. > > > > Martin > > > > On 26 April 2011 13:43, Garth N. Wells <gn...@cam.ac.uk> wrote: > > > > > > > > On 26/04/11 12:22, Martin Sandve Aln s wrote: > > > On 26 April 2011 10:56, Garth N. Wells <gn...@cam.ac.uk > > > <mailto:gn...@cam.ac.uk>> wrote: > > > > > > > > > > > > On 26/04/11 09:03, Martin Sandve Aln s wrote: > > > > See other mail. I don't see that it solves anything, it > doesn't > > seem > > > > related to anything I've read about in this thread, and it > has a > > > > potential backside in hindering the garbage collector in > Python. I > > may > > > > be wrong, but nobody has answered my other questions about > this > > > thread yet. > > > > > > > > > > As a precursor, the primary problem has nothing to do with > Instant > > disk > > > cache, etc. The Instant discussion is just confusing the > original > > point. > > > > > > In summary, is it helpful if DOLFIN can avoid calling > ufl.preprocess > > > every time a dolfin.Form object is created. DOLFIN relies on > > > preprocessing to extract the form Arguments, from which the > mesh is > > > extracted (via form_data().original_arguments, and since DOLFIN > uses > > > 'Arguments' that are subclasses of UFL and DOLFIN objects). > > > > > > The solution that Johan has implemented is to have FFC attach > the > > > form_data to a form. If a form has form_data attached, then we > know > > that > > > it has already been preprocessed. Martin won't like this > because it's > > > changing the form object. > > > > > > > > > This sounds much like my original design. Trying to recall from my > > possibly > > > rusty memory, I believe that calling myform.form_data() would > > > construct form data only the first time and the preprocessed form > could > > > be retrieved from the returned form data. The form data was > attached > > > as myform._form_data. Thus you could always say > > > preprocessed_form = myform.form_data().form > > > and preprocessing would only happen once. > > > > I think that the above would solve the issue. At the moment ufl.Form > has > > the member function: > > > > def form_data(self): > > "Return form metadata (None if form has not been > preprocessed)" > > return self._form_data > > > > If it did > > > > def form_data(self): > > if self._form_data is None: > > # compute form_data > > return self._form_data > > > > it should make things straightforward. But doesn't this violate > > immutability of the form, or is it ok since the mathematical form > itself > > is not being modified? > > > > Garth > > > > > > > This was redesigned > > > after I left to have a separate preprocess function. > > > > > > > > > It may be enough if UFL would provide a function to return a > list of > > > form Arguments, if this is fast. Something like > > > > > > def extract_original_arguments(form): > > > > > > # Replace arguments and coefficients with new renumbered > objects > > > arguments, coefficients = > extract_arguments_and_coefficients > > (form) > > > replace_map, arguments, coefficients \ > > > = build_argument_replace_map(arguments, > coefficients) > > > form = replace(form, replace_map) > > > > > > # Build mapping to original arguments and coefficients, > which is > > > # useful if the original arguments have data attached to > them > > > inv_replace_map = {} > > > for v, w in replace_map.iteritems(): > > > inv_replace_map[w] = v > > > original_arguments = [inv_replace_map[v] for v in > arguments] > > > > > > return original_arguments > > > > > > Garth > > > > > > > > > I don't understand why this is needed. We: > > > - must preprocess each form once > > > - don't want to preprocess the same form twice > > > - can obtain the original arguments after preprocessing > > > This was supported a long time ago, so unless someone has > > > removed functionality while I've been gone, what is the problem? > > > > > > I have a feeling that the source of many problems is the attempt > > > to reuse forms and change mesh, functions, or elements. > > > This is contrary to the design of UFL where expressions are > immutable. > > > > > > Martin > > > > > > > > > > Martin > > > > > > > > On 26 April 2011 09:20, Garth N. Wells <gn...@cam.ac.uk > > > <mailto:gn...@cam.ac.uk> > > > > <mailto:gn...@cam.ac.uk <mailto:gn...@cam.ac.uk>>> wrote: > > > > > > > > Martin: Any problem if we apply this patch to UFL? > > > > > > > > Garth > > > > > > > > On 25/04/11 22:50, Johan Hake wrote: > > > > > This should be fixed now. > > > > > > > > > > I do not see why we introduced the memory cache when > this > > > solution > > > > was laying > > > > > right in front our eyes... > > > > > > > > > > Anyhow. Here is a patch for ufl to avoid circular > dependency > > > between a > > > > > preprocessed form and the form_data. > > > > > > > > > > Johan > > > > > > > > > > On Monday April 25 2011 14:34:00 Anders Logg wrote: > > > > >> Simple sounds good. > > > > >> > > > > >> > > > > >> On Mon, Apr 25, 2011 at 02:29:50PM -0700, Johan Hake > wrote: > > > > >>> I am working on a simple solution, where we store > > > everything in the > > > > >>> original ufl form. > > > > >>> > > > > >>> I might have something soon. > > > > >>> > > > > >>> Johan > > > > >>> > > > > >>> On Monday April 25 2011 14:26:18 Garth N. Wells > wrote: > > > > >>>> On 25/04/11 22:08, Anders Logg wrote: > > > > >>>>> On Mon, Apr 25, 2011 at 07:40:21PM -0000, Garth > Wells > > wrote: > > > > >>>>>> On 25/04/11 20:00, Johan Hake wrote: > > > > >>>>>>> On Monday April 25 2011 11:26:36 Garth Wells > wrote: > > > > >>>>>>>> On 25/04/11 18:51, Anders Logg wrote: > > > > >>>>>>>>> On Mon, Apr 25, 2011 at 05:11:41PM -0000, Garth > > > Wells wrote: > > > > >>>>>>>>>> On 25/04/11 17:53, Johan Hake wrote: > > > > >>>>>>>>>>> On Monday April 25 2011 08:59:18 Garth Wells > wrote: > > > > >>>>>>>>>>>> On 25/04/11 16:47, Johan Hake wrote: > > > > >>>>>>>>>>>>> Commenting out the cache is really not a > fix. The > > > > problem is > > > > >>>>>>>>>>>>> within dolfin. Isn't there another way to > deal > > > with this? > > > > >>>>>>>>>>>> > > > > >>>>>>>>>>>> It is a fix if the cache isn't needed. > > > > >>>>>>>>>>> > > > > >>>>>>>>>>> Sure. > > > > >>>>>>>>>>> > > > > >>>>>>>>>>>>> First: How much penalty are there with a > > > disabled memory > > > > >>>>>>>>>>>>> cache. Maybe the problem isn't that bad? > > > > >>>>>>>>>>>> > > > > >>>>>>>>>>>> I don't get the point of this cache. The way > it > > > is now, > > > > a form > > > > >>>>>>>>>>>> is only preprocessed if it hasn't already > been > > > > preprocessed, > > > > >>>>>>>>>>>> which seems ok to me. The old code tried to > avoid > > > some > > > > >>>>>>>>>>>> preprocessing, but it was highly dubious and > I > > doubt > > > > that it > > > > >>>>>>>>>>>> was effective. > > > > >>>>>>>>>>> > > > > >>>>>>>>>>> I think the preprocessing stage actually do > take > > > some time. > > > > >>>>>>>>>>> AFAIK the preproces stage essentially do two > > > things. It > > > > >>>>>>>>>>> creates a canonical version of the Form so > two > > Forms > > > > that are > > > > >>>>>>>>>>> the same, but constructed at different times > are > > > beeing > > > > >>>>>>>>>>> treated equal wrt form generation. Then are > DOLFIN > > > specific > > > > >>>>>>>>>>> guys extracted. I am not sure what takes the > most > > > time. We > > > > >>>>>>>>>>> should probably profiel it... But if it is > the > > > latter we > > > > could > > > > >>>>>>>>>>> consider putting another cache in place which > is > > > more robust > > > > >>>>>>>>>>> wrt changing DOLFIN objects. > > > > >>>>>>>>>> > > > > >>>>>>>>>> It should be easy to avoid the overhead of > > > preprocessing by > > > > >>>>>>>>>> keeping the object in scope. If the object > changes, > > > the only > > > > >>>>>>>>>> robust way to make sure that the form is the > same > > > as one > > > > in the > > > > >>>>>>>>>> cache is to compare all the data. This > requires > > > preprocessing > > > > >>>>>>>>>> the form, which then defeats the purpose of a > cache. > > It > > > > may be > > > > >>>>>>>>>> possible to add a lightweight preprocess to > UFL, > > > but I don't > > > > >>>>>>>>>> think that it's worth the effort or extra > > complication. > > > > >>>>>>> > > > > >>>>>>> I think a light weight version might be the way > to go. > > > This > > > > is then > > > > >>>>>>> stored in memory cache. If we are able to strip > such a > > > form > > > > for all > > > > >>>>>>> DOLFIN specific things we would also prevent huge > > memory > > > > leaks with > > > > >>>>>>> mesh beeing kept. > > > > >>>>>>> > > > > >>>>>>> Then we always grab DOLFIN specific data from the > > > passed form > > > > >>>>>>> instead of grabbing from the cache. Not sure how > easy > > this > > > > will be > > > > >>>>>>> to implement, but I think we need to explore it, > as > > > the DOLFIN > > > > >>>>>>> specific part of the form really has nothing to > do > > > with the > > > > >>>>>>> generated Form. > > > > >>>>>>> > > > > >>>>>>> Martin: > > > > >>>>>>> Why is it important to have the _count in the > repr of > > the > > > > form? I > > > > >>>>>>> guess that is used in ufl algorithms? Would it be > > > possible to > > > > >>>>>>> include a second repr function, which did not > include > > > the count? > > > > >>>>>>> This would then be used when the signature is > checked > > > for. We > > > > >>>>>>> could then use that repr to generate a form which > is > > > stored > > > > in the > > > > >>>>>>> memory cache. This would then be tripped for any > > > DOLFIN specific > > > > >>>>>>> objects. This should work as the _count attribute > has > > > nothing to > > > > >>>>>>> do with what code gets generated, but it is > essential > > for > > > > internal > > > > >>>>>>> UFL algorithms, right? > > > > >>>>>>> > > > > >>>>>>>>> I'm not very happy with this change. > > > > >>>>>>>> > > > > >>>>>>>> The bright side is that slow and correct is a > better > > > starting > > > > >>>>>>>> point than fast but wrong ;). > > > > >>>>>>>> > > > > >>>>>>>> An easy fix is to attach the preprocessed form > to a > > Form > > > > object. > > > > >>>>>>>> This would work robustly if we can make forms > > > immutable once > > > > >>>>>>>> they've been compiled. Is it possible to make a > > > Python object > > > > >>>>>>>> immutable? > > > > >>>>>>> > > > > >>>>>>> We can probably overload all setattribtue methods > which > > > > prohibits a > > > > >>>>>>> user to write to these but it might not be > possible to > > > > prohibit a > > > > >>>>>>> user to change attributes on instances owned by > the > > > Form. I > > > > guess > > > > >>>>>>> this is similare to the difficulties of > preserving > > > constness in > > > > >>>>>>> C++, but I think it is even harder in Python. > > > > >>>>>> > > > > >>>>>> What if we have the FFC jit compiler return the > > > preprocessed > > > > form, > > > > >>>>>> and inside dolfin.Form simply do > > > > >>>>>> > > > > >>>>>> class Form(cpp.Form): > > > > >>>>>> def __init__(self, form, . . .. ) > > > > >>>>>> .... > > > > >>>>>> > > > > >>>>>> (...., preprocessed_form) = jit(form, . . > . . ) > > > > >>>>>> > > > > >>>>>> form = preprocessed_form > > > > >>>>>> > > > > >>>>>> ..... > > > > >>>>>> > > > > >>>>>> This way, form will have form_data, and the FFC > jit > > > function will > > > > >>>>>> know not to call ufl.preprocess. > > > > >>>>> > > > > >>>>> Here's another strange thing. In the JITObject > class, we > > > have two > > > > >>>>> functions: __hash__ and signature. As far as I > > > understand, the > > > > first > > > > >>>>> is used to located objects (generated code/modules) > in > > > the Instant > > > > >>>>> in-memory cache, while the second is used for the > > > on-disk cache. > > > > >>>>> > > > > >>>>> >From some simple tests I did now, it looks like > the > > > __hash__ > > > > function > > > > >>>>>> > > > > >>>>> does not need to any significant speedup. The JIT > > benchmark > > > > runs just > > > > >>>>> as fast if I call signature from within __hash__. > > > > >>>>> > > > > >>>>> Furthermore, the __hash__ function must also be > broken > > > since it > > > > >>>>> relies on calling id on the form. > > > > >>>>> > > > > >>>>> Ideally, we should get Instant to handle the > caching, > > both > > > > in-memory > > > > >>>>> and on-disk, by providing two functions __hash__ > (fast, > > for > > > > in-memory > > > > >>>>> cache) and signature (slow, for on-disk cache). > > > > >>>>> > > > > >>>>> Since __hash__ cannot call id, it must be able to > attach > > > a unique > > > > >>>>> string to the form (perhaps based on an internal > counter > > > in FFC). > > > > >>>>> My suggestion would be to add this to UFL, > something > > > like set_hash > > > > >>>>> and hash (which would return None if set_hash has > not > > been > > > > called). > > > > >>>>> If Martin does not like that, we should be able to > handle > > it > > > > on the > > > > >>>>> DOLFIN side. > > > > >>>>> > > > > >>>>> So in conclusion: no in-memory cache in FFC > (handled by > > > > Instant) and > > > > >>>>> FFC attaches a hash to incoming forms so that > Instant may > > > > recognize > > > > >>>>> them later. > > > > >>>> > > > > >>>> The code that I disabled was caching preprocessed > forms, > > so I > > > > don't see > > > > >>>> how this can be handled by Instant. > > > > >>>> > > > > >>>> Garth > > > > >>>> > > > > >>>>> Maybe even better: Instant checks whether an > incoming > > > object has a > > > > >>>>> set_hash function and if so calls it so it can > recognize > > > > objects it > > > > >>>>> sees a second time. > > > > >>>>> > > > > >>>>> I'm moving this discussion to the mailing list(s). > > > > >>>> > > > > >>>> _______________________________________________ > > > > >>>> Mailing list: https://launchpad.net/~ufl > > > > >>>> Post to : u...@lists.launchpad.net > > > <mailto:u...@lists.launchpad.net> > > > > <mailto:u...@lists.launchpad.net <mailto: > u...@lists.launchpad.net > > >> > > > > >>>> Unsubscribe : https://launchpad.net/~ufl > > > > >>>> More help : https://help.launchpad.net/ListHelp > > > > > > > > _______________________________________________ > > > > Mailing list: https://launchpad.net/~ufl > > > > Post to : u...@lists.launchpad.net > > > <mailto:u...@lists.launchpad.net> <mailto: > u...@lists.launchpad.net > > > <mailto:u...@lists.launchpad.net>> > > > > Unsubscribe : https://launchpad.net/~ufl > > > > More help : https://help.launchpad.net/ListHelp > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > Mailing list: https://launchpad.net/~ufl > > Post to : u...@lists.launchpad.net > > Unsubscribe : https://launchpad.net/~ufl > > 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
