← Back to team overview

dolfin team mailing list archive

Re: [HG DOLFIN] Typemap fixes?for?SubDomain::inside (), seems to work now.

 

On Wednesday 03 December 2008 22:00:35 Anders Logg wrote:
> On Wed, Dec 03, 2008 at 09:29:29PM +0100, Johan Hake wrote:
> > On Wednesday 03 December 2008 20:52:15 Anders Logg wrote:
> > > On Wed, Dec 03, 2008 at 07:01:04PM +0100, Johan Hake wrote:
> > > > On Wednesday 03 December 2008 18:04:54 Anders Logg wrote:
> > > > > On Wed, Dec 03, 2008 at 05:28:21PM +0100, Johan Hake wrote:
> > > > > > On Wednesday 03 December 2008 15:43:43 Anders Logg wrote:
> > > > > > > On Wed, Dec 03, 2008 at 03:08:32PM +0100, Anders Logg wrote:
> > > > > > > > On Wed, Dec 03, 2008 at 02:57:09PM +0100, Johan Hake wrote:
> > > > > > > > > On Wednesday 03 December 2008 13:54:47 DOLFIN wrote:
> > > > > > > > > > One or more new changesets pushed to the primary dolfin
> > > > > > > > > > repository. A short summary of the last three changesets
> > > > > > > > > > is included below.
> > > > > > > > > >
> > > > > > > > > > changeset:  
> > > > > > > > > > 5221:0e4bbb39f032405bcc4abfab501af49f6beb7b0b tag:       
> > > > > > > > > >  tip
> > > > > > > > > > user:        Anders Logg <logg@xxxxxxxxx>
> > > > > > > > > > date:        Wed Dec 03 13:54:05 2008 +0100
> > > > > > > > > > files:       demo/pde/poisson/python/demo.py
> > > > > > > > > > dolfin/swig/directors.i dolfin/swig/dolfin_mesh_pre.i
> > > > > > > > > > description:
> > > > > > > > > > Typemap fixes for SubDomain::inside(), seems to work now.
> > > > > > > > >
> > > > > > > > > Nice work!
> > > > > > > > >
> > > > > > > > > I suppose you are aware that the director typemap only
> > > > > > > > > kicks in for Function and SubDomain as these are the only
> > > > > > > > > director classes that includes a double* x, in one of their
> > > > > > > > > signatures. If other director classes in a possible future
> > > > > > > > > implements that signature, will trigger the typemap and
> > > > > > > > > that class need to implement
> > > > > > > > > geometric_dimension(). It is a nice way to solve the
> > > > > > > > > dimension problem of the numpy array, we just need to be
> > > > > > > > > aware of possible fragile side effects.
> > > > > > > >
> > > > > > > > Yes, that was the main point, to have a common typemap for
> > > > > > > > both.
> > > > > > > >
> > > > > > > > > I introduced the class specific typemap with Function in
> > > > > > > > > mind, which you now have generalized to work for any
> > > > > > > > > "double* x" used in directors. We could hypotethize that
> > > > > > > > > this could be done for more "double*" typemaps too.
> > > > > > > > >
> > > > > > > > > Most of them are now implemented as a very fragile and
> > > > > > > > > greedy numpy typemap that kicks in every where a "double *"
> > > > > > > > > is expected. This typemap depends on the user passing the
> > > > > > > > > right dimension on the array to the function. This is
> > > > > > > > > inherited from the cpp interface where you have natural
> > > > > > > > > relationship to size of arrays. But I think that it does
> > > > > > > > > not scale to python.
> > > > > > > > >
> > > > > > > > > Optimally we should use a dimension check for each double *
> > > > > > > > > typemap that expect a numpy array, where the size of the
> > > > > > > > > numpy array is defined from the contex the type map is
> > > > > > > > > applied in. E.g. a type map for the Vector.set(), could
> > > > > > > > > check the size of the array agains the size of the Vector.
> > > > > > > > > The problem with this is to make a specific typemap that we
> > > > > > > > > know only kicks in for a particular class.
> > > > > > > >
> > > > > > > > I know too little SWIG to have an opinion on this.
> > > > > > > >
> > > > > > > > > To facilitate this we could change the parameter name in
> > > > > > > > > the Vector.set(double* value) to Vector.set(double*
> > > > > > > > > vecvalue) and to add the typemap:
> > > > > > > > > (pseudo code)
> > > > > > > > > %typemap(in) double* vecvalues {
> > > > > > > > >   {
> > > > > > > > >     Check input is numpy array.
> > > > > > > > >     // This is safe as long as only Vectors implement
> > > > > > > > > "double* vecvalues" Check dimension of numpy array agains
> > > > > > > > > self->size() Assign the input statement.
> > > > > > > > >   }
> > > > > > > > > }
> > > > > > > >
> > > > > > > > But I'd like to avoid this (choosing special names for the
> > > > > > > > arguments in C++ just to please SWIG).
> > > > > > > >
> > > > > > > > > We should also %ignore alot of low level cpp only functions
> > > > > > > > > from the especially the la interface. Removing alot of the
> > > > > > > > > fragile double* typemaps that now kicks in everywhere.
> > > > > > > >
> > > > > > > > Agree!
> > > > > > > >
> > > > > > > > I'm about to get started on renaming dolfin.dolfin -->
> > > > > > > > dolfin.cpp. Let's see how it goes.
> > > > > > >
> > > > > > > Didn't go well. I don't know enough about the build system to
> > > > > > > make this work.
> > > > > > >
> > > > > > > Can you take a look?
> > > > > >
> > > > > > Yes. I do it to night.
> > > > >
> > > > > ok, nice.
> > > > >
> > > > > I'll probably try to push some changes before then.
> > > >
> > > > I have done quite abit already. You deside if you want to push.
> > >
> > > Nothing on the Python stuff so just go ahead. I'll let you push before
> > > I touch it again, probably not until tomorrow.
> >
> > Ok, I am almost finished! I needed to hack both SConscript and
> > simula-scons, but now the build system play with us.
>
> Nice!
>
> > I run into a swig bug that we previously have run into in pycc, it is
> > when compile_function %import dolfin. This statement should import the
> > package dolfin, which we rightly has set in the dolfin.i file, but
> > instead it tries to import the module, cpp, which is non excisting. The
> > fix, who the hacker Ola btw did, is to add an open string just before the
> > import statement and then the close string after, effectively commenting
> > out the wrong import statement. Then we add the correct "import dolfin"
> > manualy.
> >
> > Well who cares...
>
> It worries me that these hacks are necessary. 

This is a swig bug. I will try to report it upstream.

> It also worries me that 
> we need quite a few lines of code to just define the build:
>
>   SConstruct:        481 lines
>   dolfin/SConscript: 363 lines
>
> Shouldn't it be possible to get this down to say 50 lines, considering
> that we also depend on a special hack on top of Scons (simula-scons)
> which is itself 2000 lines (not counting the pkgconfig-generators).

He, he, you've got a point. But then we are doing something quite complex. But 
funny you mention it. I talked to Johannes today about cleaning the 
SConstruct file and SConscript files, together with a small cleanup in the 
simula-scons. 

I think it is unrealistic to get it down to 50 lines though. Only the option 
definitions are 76 lines, included outcommented lines. But the logic and 
handling of variables could have a thorough walk through. 

Also it's not many who have good knowledge about simula scons. It is probably 
Johannes, Åsmund and me. And it was built to support a many module system a 
la pycc, and dolfin is only one module, making it a bit more complex than it 
has to be. 

DOLFIN can benefit from this by getting modularized in a near future, which 
would speed up the recompilation of at least the swig wrapper files, 
previously mentioned in this list.

Johan


References