← Back to team overview

instant team mailing list archive

Re: [Dolfin] [Branch ~dolfin-core/dolfin/main] Rev 5656: Fix refinement from Python.

 

On Thu, Feb 10, 2011 at 10:05:25AM -0800, Johan Hake wrote:
> On Thursday February 10 2011 09:36:18 Anders Logg wrote:
> > On Thu, Feb 10, 2011 at 06:19:59AM -0800, Johan Hake wrote:
> > > On Thursday February 10 2011 02:11:42 Johannes Ring wrote:
> > > > On Thu, Feb 10, 2011 at 10:48 AM, Anders Logg <logg@xxxxxxxxx> wrote:
> > > > > On Thu, Feb 10, 2011 at 09:43:16AM +0000, Garth N. Wells wrote:
> > > > >> On 10/02/11 09:39, Anders Logg wrote:
> > > > >> > On Thu, Feb 10, 2011 at 09:22:39AM +0000, Garth N. Wells wrote:
> > > > >> >> On 10/02/11 09:19, Anders Logg wrote:
> > > > >> >>> On Thu, Feb 10, 2011 at 09:11:26AM +0000, Garth N. Wells wrote:
> > > > >> >>>> On 10/02/11 09:09, Anders Logg wrote:
> > > > >> >>>>> On Thu, Feb 10, 2011 at 08:48:55AM +0000, Garth N. Wells wrote:
> > > > >> >>>>>> On 10/02/11 07:38, Johan Hake wrote:
> > > > >> >>>>>>> On Wednesday February 9 2011 23:32:55 Anders Logg wrote:
> > > > >> >>>>>>>> On Wed, Feb 09, 2011 at 04:40:19PM -0800, Johan Hake wrote:
> > > > >> >>>>>>>>> Nice fix!
> > > > >> >>>>>>>>>
> > > > >> >>>>>>>>> I thought we needed to introduce shared_ptr versions of
> > > > >> >>>>>>>>> refinements call. But I realise this fix only work for
> > > > >> >>>>>>>>> SWIG 2.0. As all shared_ptr_foo are renamed to foo.
> > > > >> >>>>>>>>>
> > > > >> >>>>>>>>> We need to add an extra layer of %rename/%ignore for the
> > > > >> >>>>>>>>> SWIG 2.0 case.
> > > > >> >>>>>>>>>
> > > > >> >>>>>>>>> I can do this. But again it introduces another layer of
> > > > >> >>>>>>>>> complexity you mention in your other post.
> > > > >> >>>>>>>>
> > > > >> >>>>>>>> That didn't seem to work. The buildbot now says:
> > > > >> >>>>>>>>
> > > > >> >>>>>>>> File
> > > > >> >>>>>>>> "/home/fenicsslave/jhbuildbot/fenics/lib/python2.6/site-pac
> > > > >> >>>>>>>> kage s/dolfin/mes h/refine.py", line 30, in refine
> > > > >> >>>>>>>>
> > > > >> >>>>>>>>   return mesh.child_shared_ptr()
> > > > >> >>>>>>>>
> > > > >> >>>>>>>> TypeError: in method 'HierarchicalMesh_child_shared_ptr',
> > > > >> >>>>>>>> argument 1 of type 'dolfin::Hierarchical< dolfin::Mesh > *'
> > > > >> >>>>>>>>
> > > > >> >>>>>>>> What does that mean? mesh.child_shared_ptr() should return
> > > > >> >>>>>>>> a shared_ptr to an object of class T (in this case Mesh),
> > > > >> >>>>>>>> which it does in the C++ interface. It should not return a
> > > > >> >>>>>>>> pointer to an object of class Hierarchical<T>.
> > > > >> >>>>>>>
> > > > >> >>>>>>> It is related to the "fix" Garth introduced. He used
> > > > >> >>>>>>> foo_shared_ptr, but that one does not work for SWIG < 2.0.
> > > > >> >>>>>>> These methods are now ignored or renamed for all SWIG
> > > > >> >>>>>>> versions, and everything should be well and fine for both
> > > > >> >>>>>>> versions of SWIG.
> > > > >> >>>>>>
> > > > >> >>>>>> I don't think that this is really a fix. The Python refine
> > > > >> >>>>>> should return a shared_ptr, otherwise we can have problems
> > > > >> >>>>>> with things going out of scope. The underlying problem is
> > > > >> >>>>>> SWIG not wrapping the Mesh shared_ptrs properly.
> > > > >> >>>>>
> > > > >> >>>>> But isn't that exactly what's happening now? The C++
> > > > >> >>>>> shared_ptr_foo() functions are renamed to foo() so when you
> > > > >> >>>>> call foo() in Python you get the shared_ptr.
> > > > >> >>>>
> > > > >> >>>> That was my fix, which was reverted. Looks to me like a
> > > > >> >>>> reference is returned.
> > > > >> >>>
> > > > >> >>> From site-packages/dolfin/mesh/refine.py:
> > > > >> >>>   return mesh.child()
> > > > >> >>>
> > > > >> >>> From dolfin/swig/shared_ptr_classes.i:
> > > > >> >>>    boost::shared_ptr<DERIVED_TYPE> child()
> > > > >> >>>    { return self->child_shared_ptr(); }
> > > > >> >>>
> > > > >> >>> So when mesh.child() is called in refine.py, it really returns
> > > > >> >>> mesh.child_shared_ptr().
> > > > >> >>
> > > > >> >> OK.
> > > > >> >>
> > > > >> >> It' pretty confusing having stuff dotted around in different
> > > > >> >> files. We'll be able to remove it when we moving to SWIG 2.
> > > > >> >
> > > > >> > Is it the case that it won't be in Ubuntu 11.04 and we will have
> > > > >> > to wait until 11.10?
> > > > >>
> > > > >> It will be in 11.04, but the executable will be called swig2.0.
> > > > >>
> > > > >> The sooner we can move, the better. Saves maintaining and testing
> > > > >> double SWIG code.
> > > > >
> > > > > Agree. The new name of the executable shouldn't be a problem.
> > > > >
> > > > > Isn't it just a matter of (i) some fixes to the buildsystem for
> > > > > DOLFIN (which I'm sure Johannes can handle) and (ii) some fixes to
> > > > > Instant and the UFC wrapper utils?
> > > >
> > > > It is only Instant that must be fixed.
> > >
> > > It's on my todo list
> >
> > Great. So the conclusion is we move to SWIG 2.0 after the release of
> > Ubuntu 11.04?
>
> And after DOLFIN 0.9.10 I would say. I am pretty loaded for the moment...

0.9.10 should come before 11.04 so that's no problem. The problem was
to make the release this week. It may still happen but there are some
things to sort out.

--
Anders



References