dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

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 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++.

-- 
Anders

Attachment: signature.asc
Description: Digital signature