ffc team mailing list archive

[Johan Hake <johan.hake@xxxxxxxxx>]

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

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: hake.dev@xxxxxxxxx-20110425214056-1g2oic83wikpjdaj
# target_branch: bzr+ssh://bazaar.launchpad.net/~ufl-core/ufl/main/
# testament_sha1: 48930d933e8fde5c7b4b1b7b1984a67c3e4f23d8
# timestamp: 2011-04-25 14:42:54 -0700
# base_revision_id: gnw20@xxxxxxxxx-20110415200519-tga4kz0pgu0sll18
# 
# Begin patch
=== modified file 'ufl/form.py'
--- ufl/form.py	2011-04-12 13:03:47 +0000
+++ ufl/form.py	2011-04-25 21:40:56 +0000
@@ -95,6 +95,12 @@
         "Return form metadata (None if form has not been preprocessed)"
         return self._form_data
 
+    def __del__(self):
+        if self._form_data is not None:
+            # Delete preprocessed form from form_data, preventing memory
+            # leak cause by circular referencing
+            del self._form_data._form
+    
     def __add__(self, other):
         # --- Add integrands of integrals with the same measure
 

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWeE8UG8AAZrfgERQWGf/93ZR
BAC////wUAPe9soAXYAAyqepsU2U9QDT0QaMgGgAAANU2pGTNTTR6npqMEaGhkGTIGjIxzAAAAAA
AAAABRqntUYjahkNDaTJkYRhHqekGjTCSQI0ARpkmEyGp6TU8EjQekbSQ1DI2biH39zPTT+Hbz59
9xLY2oDbfW/hjZqhKCV1jgRyFza6UmIYy1E+HxoQBsQ6QJ3Syy7YpT7GQAXvETgXYIhyg8scqNl1
XlC6uZEA7BIsd0BdRcMWJ0b58W0opEaBM9Nl0xY1m7ctwuPwtfbt1qfqu7s5RvwxLlilQlB671s2
4XUQF39E71QD+BN3FzOEXYKItzk5TIqlN8a4Shsh+l7NYiehCwQ8LfhaCGmRZUuwqymK07oEAuMr
GG6dUPWKzVitocVPuIapn2FSmTMI+VN4slsxUsBPWq42XY1hv/YU+6yjXQXKc3EYMNZ01HKYq6dT
KlBMQ/YWqbs1eaUkSnlw3uM4suatLGXG3CtWK8DKgwTFhAkLaub3KDD8tz6B7JVa7NpeqC6gjhlf
jSsWLDEzIYbJ3VxVVSti0cSzU50bcRS36rCumzKp6qv+V3qCnknKO0YtNRkWlvs1tdarWd+Wii+T
nvHOTSaBQEqGy9rW8Z2Gzexv+/jeuxLLjZUDHxc9oMeZIcTB5zUrsHB7ebylmGU9IB+5l4pj04Wq
ifnb9bvINSW4UT8YJa/TyTrIT8dhcixWA2dTeXSnr4W8KO0JKuh3PQ1i/xU57Pb3JD2m4kOfvSuj
oTiyMcsd0bzCadwknxXoGcDZ4fl4pFyKImUNfPouTSvzqJgkp3hHeDU0YzMy/nytwDKTroG8hxjS
MCe2IMbFBTxjn3LILa2wbr5IYR8zLPBtBkcAZEtM9FT29tW0ametyTkTULqJLLjmYZQ2nXEqVS/G
7v8jh9Hnv7s94zHmzNFWi2L6i9EpLet4cAsLBo3/V0v5yY/Pyslngcqhbx2T/60TS/3vPC4W1LBS
M92DP8wfkP/7TSMKQrVBPLqHWXW9Hgu0Tifcmcg+NoeAui7dHpOCtWAzgpYqUVgUQCNC10Ut2nFm
FJOTmD1S0coVj7DXRYpRIjme0FzTDqIRYqgERg5faiQoMmViw3Hdeqvz6i3w7AKkfPf+IlYWIXHR
7MXXPkDY6MycQ810FPS9UrYTzWioW1RVYcpkwgookPafB5HlNCkatcHv5qVVHNMiNg3OWLjpaXrG
79qgdUdWzVgUgw193Y5v0OCi/2pjC8+mCpq4WDWc6S0Hg04vVOL10WPpXxXK04Ws1eK1pYtemCDl
icdgvFSWrkxyTEauarzNs//F3JFOFCQ4TxQbwA==