dolfin team mailing list archive

[Johan Hake <hake@xxxxxxxxx>]

On Friday 22 August 2008 10:25:01 Anders Logg wrote:
> On Fri, Aug 22, 2008 at 10:08:07AM +0200, Martin Sandve Alnæs wrote:
> > 2008/8/22 Anders Logg <logg@xxxxxxxxx>:
> > > On Thu, Aug 21, 2008 at 11:43:41PM +0200, Johan Hake wrote:
> > >> On Thursday 21 August 2008 23:05:57 Evan Lezar wrote:
> > >> > Hi
> > >> >
> > >> > I added a simple warning message to the Function constructor in
> > >> > function.py which is displayed if the constructor is called with 3
> > >> > arguments and the last one is not an instance of dolfin.Vector or
> > >> > Matrix.
> > >> >
> > >> > I know that a warning is not a necessarily a long-term solution, but
> > >> > it would at least assist new users such as myself in tracking down
> > >> > the source of errors in their code.
> > >>
> > >> Fine that you found out what your problem was!
> > >>
> > >> Your issue again illustrates the limitation Function in pydolfin. We
> > >> should really update at least the docstrings, together with more
> > >> exstensive testings. For example could these lines be expanded:
> > >>
> > >>      # Otherwise give all to DOLFIN
> > >>      else:
> > >>          dolfin.cpp_Function.__init__(self, *((element,) + others))
> > >>
> > >> so that we only send valid arguments to the cpp_Function constructor.
> > >>
> > >> A while ago I implemented a way of instantiate a discrete function
> > >> using a numpy array. I promised Martin to send a patch to him for
> > >> inclusion but faild to do it. Please apply Evans patch then I can hand
> > >> a patch for the numpy array initialization after that.
> > >
> > > Done.
> >
> > This check is wrong in many ways:
> >
> >         # Check arguments
> >         if len(others) == 2:
> >             if not isinstance(others[1], (dolfin.Vector,
> > dolfin.GenericVector)): error("Coefficients should be dolfin Vector or
> > Matrix")
> >
> > First, the len(others) == 2 is a very very ugly hack that hides
> > some of the dolfin::Function::Function signatures.
> >
> > Second, checking for dolfin.Vector is superfluous and misleading for
> > people reading the code (they might repeat the practice and forget
> > the GenericVector part in other code).
> >
> > Third, the term "Coefficients" in the message has no clear no meaning.
> >
> > Fourth, "or Matrix" should of course be removed.
> >
> > I'll leave it up to those who use dolfin.Function to fix this mess the
> > way they want it.
>
> Yes, you are right. The Function class in PyDOLFIN is a hack. It needs
> to be fixed, but probably won't before UFL is ready.
>
> But other than the first special case where we need to create a form
> to build a dof map, the two other cases look fine to me:
>
>   # If we have an element, then give element to FFC and the rest to DOLFIN
>   elif isinstance(element, (ffc.FiniteElement, ffc.MixedElement)):
>       ffc.Function.__init__(self, element)
>       dolfin.cpp_Function.__init__(self, *others)
>   # Otherwise give all to DOLFIN
>    else:
>       dolfin.cpp_Function.__init__(self, *((element,) + others))
>
> SWIG should take care of checking the arguments (*others) against the
> constructors of Function in DOLFIN C++. I don't know what goes wrong,
> but it seems a typemap gets in the way.

I think that it have to be the "python sequence" to Array<uint/double> typemap 
I added by request from Martin that kicks in. 

If we add a check for numpy array in addition to GenericVector and Vector in 
the first test, and then create a Vector from the numpy array, we can avoid 
this. 

Shall I send a patch?

Johan

> I have removed the check. You are right that we shouldn't need to
> manually check against all variants of constructors that we have
> defined for Function in C++.