dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Tue, Dec 02, 2008 at 10:35:31PM +0100, Johan Hake wrote:
> On Tuesday 02 December 2008 15:42:53 Anders Logg wrote:
> > On Tue, Dec 02, 2008 at 12:12:14PM +0100, Anders Logg wrote:
> > > I've tried adding a new class Constant in function.py:
> > >
> > > class Constant(ffc.Constant, dolfin.cpp_Function):
> > >
> > >     def __init__(self, domain, value):
> > >         "Create constant-valued function."
> > >         print domain
> > >         print value
> > >         #ffc.Constant.__init__(self, domain)
> > >         #dolfin.cpp_Constant.__init__(self, value)
> > >
> > > But I get the following error message:
> > >
> > >   File
> > >  
> > > "/scratch/fenics/dolfin/dolfin-dev/local/lib/python2.5/site-packages/dolf
> > >in/function.py", line 411, in <module> class Constant(ffc.Constant,
> > > dolfin.cpp_Function):
> > > TypeError: Error when calling the metaclass bases
> > >     function() argument 1 must be code, not str
> > >
> > > How is this possible? There should be no metaclasses involved here
> > > (except the built-in Python metaclass type that is always there).
> 
> Meta classes is always involed when you try to create a class. This time it is 
> not the FunctionCreator meta class, even if you first think so.
> 
> If you ever tried to subclass a python function, which you are trying, you 
> will get the same error:
> 
>   >>> def jada():pass
>   ...
>   >>> class Jada(jada):pass
>   ...
>   Traceback (most recent call last):
>     File "<stdin>", line 1, in <module> 
>   TypeError: Error when calling the metaclass bases
>       function() argument 1 must be code, not str
> 
> ffc.Constant is a function not a class...

Sorry, my fault. It looks like someone did a clever trick when
implementing Constant in FFC as a function and not a class... :-)

> > I get this error even if I just try to create a class named anything
> > that inherits from ffc.Constant.
> >
> > Does the metaclass construction in function.py have side-effects?
> 
> Well, for it's use I think it works fine. The user should never bother with 
> the metaclass, as the newly added quote to the docstring is mentioning. 
> 
> What we need to do is to assert the user use the Function class correct. I 
> have tried to put in some checks with hopefully informative error messages. 
> But I dont know if I have covered all bases. We need user feedback for this. 

I'm sure the checks are very good, but could the logic be broken up
to first split on the different types, so there's essentially one case
for each of the different options in the Function docstring, and then
a function is called for each case to do the work?

> > I don't remember if we discussed this before, but would it be possible
> > (at least simpler) to instead define a simple Python function that
> > returns a "function" instance:
> >
> > class FooFunction(ffc.Function, ...):
> >     ...
> > class BarFunction(dolfin.Function, ...):
> >     ...
> >
> > def Function(V, *arg):
> >
> >     if foo:
> >         return FooFunction(...)
> >     elif bar:
> >         return BarFunction(...)
> >
> > This seems to be an easier solution. It would still be dynamic.
> 
> This is mre or less what is going on right now. Instead of predefining the 
> different classes they are defined dynamically in the __new__ function. We 
> need to do this because the compiled functions are created at runtime. 
> 
> I can try to untangle the code a bit as asked for previously. The suggestion 
> below which will extract the instantiation of compiled functions and discrete 
> functions is one more radical example.

ok.

> > The only drawback would be that we can't do
> >
> >   isinstace(v, Function)
> 
> This is not possible now either, as we are dynamically creating classes in 
> __new__, and returning instances of other classes than Function
> 
> Basically we are now dynamically creating Function classes that are all having 
> the name "Function" but they are not of the same class. This can be 
> illustrated by this:
> 
>   class Function(object):
>       def __new__(cls,cppexpr=None):
>           if cppexpr is None:
>               return object.__new__(cls)
>           class CompiledClass(object):pass
>           class Function(CompiledClass):pass
>           return Function()
> 
>   f  = Function()
>   f2 = Function(cppexpr="")
> 
>   isinstance(f,Function)  # This will return True
>   isinstance(f2,Function) # This will return False
> 
> I cannot see how we can avoid this, as we need to create classes at runtime. 
> As you mention we could add a FunctionBase class that we could check against. 
> But I was convinced by Martins argument previously that in PyDOLFIN code we 
> never want to check if a function is both a cpp_Function and a ffc.Function, 
> but rather for one of them. We could write this as a note for developers of 
> PyDOLFIN, and live with it. 
> 
> I am a bit more worried for advanced python users that do not know of this. 
> They could write user code that include isinstance(f,Function) statements, 
> and it wont behave as expected. 
> 
> We could add a 
> 
>   CompileFunction(cppexpr=None, cppcode=None)
> 
> which behave in the same manner as Function(cppexpr/cppcode) does today. In 
> this way we can dynamically create functions that inherits, 
> cpp_JitCompiledFunction, ffc.Function _and_ dolfin.Function, where the latter 
> is a class which is either an empty class or one that holds the 
> FunctionSpace. If we do this we also need to have an additional 
> function/class to instantiate discrete functions. 
> 
> This way to handle the isinstance problem can also be implemented for all 
> cases where Function is subclassed, i.e., both for Jit compiled and ordinary 
> python Functors.
> 
> But I have a feeling we add more complexity than we remove...

If I understand this correctly, it seems to be a simplification, or at
least it makes it more explicit which we all know is better (import
this).

But I would like a Function class (or function) on top that delegates
the creation of Function objects to these classes.

> > This can be solved by creating an empty class FunctionBase that all
> > the special types inherit from.
> 
> This is the easiest way to fix the isinstance problem, but I think it is a bit 
> unintuitive to check for FunctionBase when you have created a Function 
> object.
> 
> So I have scretched three possible ways to have it:
>   
>   1) As it is now (with clearer code). Everything is handled with
>      the Function class. You cannot check for isinstance(f,Function)
> 
>   2) Add CompiledFunction and DiscreteFunction. You can now 
>      check for isinstance(f,Function) everywhere.
> 
>   3) Add FunctionBase, you can now check for isinstance(f,FunctionBase)
> 
> I go for either 1 or 2.

I'd like all of the above! :-)

1. One single Function class using the metaclass trick to return
instances with dynamic inheritance.

2. Add classes CompiledFunction, DiscreteFunction etc that handle all
the different ways in which the C++ part of a Function can behave.

The created instances will inherit from ffc.Function (later
ufl.Function) and one of these classes.

3. Add a class FunctionBase that all these classes inherit from.

Then there will be a common interface Function for all different types
of functions, the implementation will be separated into manageable
pieces and we can use isinstance(f, FunctionBase).

-- 
Anders

Attachment: signature.asc
Description: Digital signature