dolfin team mailing list archive

[Johan Hake <johan.hake@xxxxxxxxx>]

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 ;) 

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

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

> 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.
 
> > 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. If you ever took a 
look into std_vector.i you would see _a lot_ of un-understandable things too.

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

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

Johan

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