← Back to team overview

dolfin team mailing list archive

Re: Patch for pyDolfin function.py

 

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

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.

--
Martin


Follow ups

References