dolfin team mailing list archive

["Martin Sandve Alnæs" <martinal@xxxxxxxxx>]

2008/8/22 Johan Hake <hake@xxxxxxxxx>:
> Missed the list.
>
>
> ---------- Videresendt melding ----------
> From: "Evan Lezar" <evanlezar@xxxxxxxxx>
> To: "Johan Hake" <hake@xxxxxxxxx>
> Date: Fri, 22 Aug 2008 11:16:23 +0200
> Subject: Re: [DOLFIN-dev] Patch for pyDolfin function.py
>
>
> On Fri, Aug 22, 2008 at 11:12 AM, Johan Hake <hake@xxxxxxxxx> wrote:
>>
>> 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<>?

For most cases it should be possible, but it will be ambiguous for some (very)
special cases like when assembing over a single cell for testing and having
a vector valued coefficient function of length equal to the dimension of the
global finite element space. I guess that's only possible with DG0
vector elements?


>> > > 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?
>
> Actually the docstrings are not such a bad idea.  I always have a look at what the docstring says before calling a function (? in ipython).

Docstrings are not only a good idea, they are vital for this kind of argument
interpretation to be even remotely usable by non-developers.

In my opinion, the clearest (and most pythonic?) way to fix
these ambiguities would be to use keyword arguments.

-- 
Martin