dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

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

-- 
Anders

Attachment: signature.asc
Description: Digital signature