dolfin team mailing list archive

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

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.

> Shall I send a patch?

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

--
Martin