← Back to team overview

ffc team mailing list archive

Re: [Ufl] [Bug 769811] [NEW] JIT cache problem with id(form)

 


On 26/04/11 12:22, Martin Sandve Alnæs wrote:
> On 26 April 2011 10:56, Garth N. Wells <gnw20@xxxxxxxxx
> <mailto:gnw20@xxxxxxxxx>> 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 <gnw20@xxxxxxxxx
>     <mailto:gnw20@xxxxxxxxx>
>     > <mailto:gnw20@xxxxxxxxx <mailto:gnw20@xxxxxxxxx>>> 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.
>     >     >>
>     >     >> --
>     >     >> 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
>     <mailto:ufl@xxxxxxxxxxxxxxxxxxx>
>     >     <mailto:ufl@xxxxxxxxxxxxxxxxxxx <mailto:ufl@xxxxxxxxxxxxxxxxxxx>>
>     >     >>>> Unsubscribe : https://launchpad.net/~ufl
>     >     >>>> More help   : https://help.launchpad.net/ListHelp
>     >
>     >     _______________________________________________
>     >     Mailing list: https://launchpad.net/~ufl
>     >     Post to     : ufl@xxxxxxxxxxxxxxxxxxx
>     <mailto:ufl@xxxxxxxxxxxxxxxxxxx> <mailto:ufl@xxxxxxxxxxxxxxxxxxx
>     <mailto:ufl@xxxxxxxxxxxxxxxxxxx>>
>     >     Unsubscribe : https://launchpad.net/~ufl
>     >     More help   : https://help.launchpad.net/ListHelp
>     >
>     >
> 
> 




Follow ups

References