← Back to team overview

dolfin team mailing list archive

Re: Patch for pyDolfin function.py

 

On Friday 22 August 2008 11:03:04 Martin Sandve Alnæs wrote:
> 2008/8/22 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.
>
> Seems right.
>
> > 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.
>
> No, that's not correct. The constant vector function constructor
> should take numpy arrays or tuples, not Vector.

Is it possible to check the length of the numpy array and compare this with 
the type of element and the size of the mesh to and try to "figure" out if 
the numpy array should be a Vector() or Array<>?

> > Shall I send a patch?
>
> It's not really a big deal, is it? I say let it be until it's fixed
> properly.

Well, sending a numpy array to a Function is quite intuitive for initialize a 
discrete function, at least for first timers that is used to numpy and 
Python. And the result Evan got was definetly not a nice error for this 
guess.

We could add in the docstring that if you send a Vector you create a discrete 
function and if you send a numpy sequence then you create a constant 
function?

Johan


Follow ups

References