dolfin team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

Johan Hake wrote:
> On Monday 30 November 2009 19:01:25 Garth N. Wells wrote:
>> Johan Hake wrote:
>>> On Monday 30 November 2009 17:22:19 Garth N. Wells wrote:
>>>> Anders Logg wrote:
>>>>> On Mon, Nov 30, 2009 at 10:57:17PM +0000, Garth N. Wells wrote:
>>>>>> Johan Hake wrote:
>>>>>>> On Monday 30 November 2009 12:11:34 Anders Logg wrote:
>>>>>>>> On Sun, Nov 29, 2009 at 09:51:25PM +0000, Garth N. Wells wrote:
>>>>>>>>> Anders Logg wrote:
>>>>>>>>>> On Sat, Nov 28, 2009 at 11:26:07PM +0100, Anders Logg wrote:
>>>>>>>>>>> On Sat, Nov 28, 2009 at 11:23:58PM +0100, Anders Logg wrote:
>>>>>>>>>>>> On Sat, Nov 28, 2009 at 10:21:18PM +0000, Garth N. Wells wrote:
>>>>>>>>>>>>> Anders Logg wrote:
>>>>>>>>>>>>>> On Sat, Nov 28, 2009 at 09:32:03PM +0000, Garth N. Wells wrote:
>>>>>>>>>>>>>>> It would be good to make a release of DOLFIN/FFL/UFL next
>>>>>>>>>>>>>>> week with the new syntax for Constants and Expressions. Are
>>>>>>>>>>>>>>> there any pressing issues which need to be addressed before
>>>>>>>>>>>>>>> making a new release?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Garth
>>>>>>>>>>>>>> I agree. Let's make a release as soon as possible.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The only things I see missing are
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1. std::vector argument in eval. I see you've started on this.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2. Getting the buildbot running in on form or another.
>>>>>>>>>>>>> If we don't get this running in time, I'm happy if we run the
>>>>>>>>>>>>> tests by hand on a few OSes.
>>>>>>>>>>>> Me too.
>>>>>>>>>>>>
>>>>>>>>>>>>>> Andre Massing has prepared a major bundle on the CGAL stuff
>>>>>>>>>>>>>> but that can wait until after 0.9.5, but it would be good to
>>>>>>>>>>>>>> do it immediately after so we get that done.
>>>>>>>>>>>>> Perhaps he could publish it first as a personal branch on
>>>>>>>>>>>>> Launchpad?
>>>>>>>>>>>> Yes, it would be a good opportunity to test that feature.
>>>>>>>>>>>>
>>>>>>>>>>>> What do you think Andre? Could you give it a try?
>>>>>>>>>>> Another thing to figure out is the logic/algorithm for selecting
>>>>>>>>>>> coefficient element degrees.
>>>>>>>>>>>
>>>>>>>>>>> We have another thread going on this.
>>>>>>>>>> Another thing that we might want to fix in the new release is the
>>>>>>>>>> ability to do
>>>>>>>>>>
>>>>>>>>>>   return (foo, bar)
>>>>>>>>>>
>>>>>>>>>> instead of
>>>>>>>>>>
>>>>>>>>>>   values[0] = foo
>>>>>>>>>>   values[1] = bar
>>>>>>>>>>
>>>>>>>>>> in the Expression class in Python.
>>>>>>>>>>
>>>>>>>>>> Johan hinted that it would be possible to implement this.
>>>>>>>>>>
>>>>>>>>>> On the other hand, one can argue that the simplified Expression
>>>>>>>>>> interface (using C++ string expressions) is already simple enough
>>>>>>>>>> for simple cases and that one should need to assign to values when
>>>>>>>>>> subclassing Expression to make it consistent with the C++
>>>>>>>>>> interface.
>>>>>>>>>>
>>>>>>>>>> Opinions?
>>>>>>>>> I like to keep the consistency with C++, plus Expressions which
>>>>>>>>> demand a subclass in place of JIT are usually reasonably
>>>>>>>>> complicated, so it may in practice be more like
>>>>>>>>>
>>>>>>>>>   return (............................................,
>>>>>>>>> ......................................)
>>>>>>>> Agreed. Let's keep the eval interface as is.
>>>>>>>>
>>>>>>>> What remains before a release? I can see these two:
>>>>>>>>
>>>>>>>> 1. Getting the la unit tests (get_row) working. Are you working on
>>>>>>>> this Johan?
>>>>>>> Yes.
>>>>>>>
>>>>>>> At least the amount of time I feel correct using on it. It is a bit
>>>>>>> more nasty than I first anticipated. But I have good hope!
>>>>>> Would these difficulties be resolved if we use the native SWIG
>>>>>> wrappers for std::vector instead of homemade versions case-by-case? I
>>>>>> know that the SWIG std::vector wrapper was removed to reduce the size
>>>>>> of the wrapper code, but perhaps the same could be achieved by using
>>>>>> more %ignore?
>>>>>>
>>>>>> Garth
>>>>> If the built-in wrappers work, I suggest we use them instead of the
>>>>> our homemade versions even if the code increases if that makes the
>>>>> wrapper code easier to maintain.
>>>> I've had a look at this. Using the SWIG wrappers I've deleted tons of
>>>> our complicated wrapper code, and things that didn't work now do
>>>> automatically. I'll push a first go soon.
>>> I'am not at all satisfied with this change. I would have hoped that you
>>> at least could wait for my response on this issue, as the amount of hours
>>> of both payed and free time, I have put down on the code is quite large.
>>> I do not want to be grumpy, but I think the point is legitimate, as
>>> DOLFIN is a community project, and this change really turned me off.
>> That's what we have 'bzr revert' for. ;) Fell free to change it back if
>> you have a manageable solution.
> 
> Sure, I was about to commit this evening but this somewhat turned me off ;) 
>

Are you going to push it?

>>> I see your point of having a manageable SWIG interface. It's a strong
>>> one. However the lack of compatibility with NumPy still exists.
>> Maintainability has to be the overriding consideration. 
> 
> Yes, I also think this is a strong goal, and I have therefore spent a lot of 
> time documenting the interface files. 
>

The documentation helps me a lot in how to use something, but it hasn't
helped (and nor could it) when making changes.

>> I have spent a
>> lot of time trying (and sometimes occasionally succeeding) to make just
>> small updates and changes. The time required is not sustainable. 
> 
> No I agree that std::vector and home made typemaps was a bit over the top. But 
> the change to std::vector was a decision that was taken in a thread, with no 
> or few considerations for the SWIG interface.
>

We actually had SWIG/Python in mind when making the change, the idea at
the time being that by using std::vector, x would carry information on
its dimension which was previously inferred by through the Mesh. I'm not
sure that this is valid still in the light of recent changes.

>> Also,
>> the difficulty of working with the SWIG interface will make it near
>> impossible to engage new contributors/maintainers.
> 
> Sure, I just think it was a bit drastic to just remove it instead of having a 
> discussion on a Blueprint or something.
>

Now that I think about Launchpad, I should have used the fancy new
feature to publish a branch along side dolfin/main.

>>> NumPy is the natural choice when one want to use contiguous arrays back
>>> and forth to Python. I would say that a strong design goal is to have a
>>> Python interface that works smoothly with NumPy, which neither
>>> std::vector nor std_vector.i do.
>> Is there a fundamental reason why std::vector and can't NumPy play well
>> together? 
> 
> Data ownership is not loose enough in std::vector to play well with NumPy 
> arrays. I think this is the main issue. Ola might add more here? I think it 
> would be natural for Ola to weight in on this issue before such a change. 
> 
>> Since vectors use contiguous arrays, sounds to me like there
>> is scope for adding a numpy.i file to SWIG.
> 
> There are such a file, however it works with (int n, double* data) and not 
> std::vector.
> 
>> It would be nice to have the NumPy integration, but the typemaps are too
>> complicated for us mere mortals.
> 
> Yes, and FE coding is way too complicated for me to know all the nit-bits in 
> so I am glad there are other people taking care of this. 

Importantly, there is more than one person active on the FE side. We
need more than one person with the ability to work on the SWIG and
Python sides. In the absence of able people, we need to make it simple
enough that a least a few of us can take care of it.

If you ever took a
> look into std_vector.i you would see _a lot_ of un-understandable things too.
> 

Yep, but I don't have to look, just like I don't read the STL file
'vector'. I just use it and it works

> I think we had a manageable interface before the std::vector change. But 
> that's just my 2 cent.
> 

For better or worse, we've generally followed a principle of designing
what we think is the best C++ interface and then let SWIG and Python
follow. It's worked well so far, with a some C++ constructs added and
designs modified designs to make the Python side work ultimately being
unnecessary and therefore removed.

>>> A better solution, in my point, would either be to stay with double/uint
>>> * for the communication of contiguous arrays, or to add a light weight
>>> wrapped class in C++ for these data structures.
>> From the C++ side, I would like to see plain pointers removed from the
>> user interface.
> 
> Yes, and I agree. It is preferable from a Python side of view too. But I would 
> prefer it over a std::vector interface in Python anytime. There might also be 
> other solutions, which we have not tried. Heck, for all I know, going with 
> std_vector.i might be the best overall solution, but I am _really_ not 
> convinced.
> 

I'm wondering if std::vector and plain pointers in the interface are the
right thing to be wrapping with NumPy. We're generally just exchanging
coordinate data and function values in and out. Should it instead be
Vectors and Matrices that interface to NumPy? We can access the raw
pointers for these objects to create NumPy objects, and we can create
Vectors and Matrices from pointers, for example using

  VecCreateSeqWithArray
  MatCreateSeqAIJWithArrays

Whatever the case may be, things have been broken for some days now so
we settle on a fix to get back on track.

Garth

> Johan
> 
>> Garth
>>
>>> Johan
>>>
>>>> Garth
>>>>
>>>>> --
>>>>> Anders