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

From: Johannes Ring <[email-masked]>
Date: Thu, 10 Feb 2011 11:11:42 +0100

On Thu, Feb 10, 2011 at 10:48 AM, Anders Logg <[email-masked]> 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-packages/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.

Johannes

>> > And what's the status of the Python Trilinos wrappers? Do we need
>> > them? I guess there's a small chance that Trilinos will move to 2.0
>> > any time soon.
>> >
>>
>> The next PyTrilinos release will require SWIG 2.0. It should also fix
>> the MPI finalisation issues.
>
> ok.
>
> --
> Anders
>
> _______________________________________________
> Mailing list: https://launchpad.net/~dolfin
> Post to     : [email-masked]
> Unsubscribe : https://launchpad.net/~dolfin
> More help   : https://help.launchpad.net/ListHelp
>

---

From: Johan Hake <[email-masked]>
Date: Thu, 10 Feb 2011 06:19:59 -0800

On Thursday February 10 2011 02:11:42 Johannes Ring wrote:
> >> >>>>>>>>> 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.
> >> >>>>>>>>> 
> >> >>>>>>>>> I can do this. But again it introduces another layer of
> >> >>>>>>>>> complexity you mention in your other post.
> >> >>>>>>>> 
> >> >>>>>>>> "/home/fenicsslave/jhbuildbot/fenics/lib/python2.6/site-package
> >> >>>>>>>> s/dolfin/mes h/refine.py", line 30, in refine
> >> >>>>>>>> TypeError: in method 'HierarchicalMesh_child_shared_ptr',
> >> >>>>>>>> argument 1 of type 'dolfin::Hierarchical< dolfin::Mesh > *'
> >> >>>>>>>> 
> >> >>>>>>>> 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.
> >> >>> 

It's on my todo list

Johan

---

From: Anders Logg <[email-masked]>
Date: Thu, 10 Feb 2011 18:36:18 +0100

> > It is only Instant that must be fixed.

Great. So the conclusion is we move to SWIG 2.0 after the release of
Ubuntu 11.04?

--
Anders

---

From: Johan Hake <[email-masked]>
Date: Thu, 10 Feb 2011 10:05:25 -0800

On Thursday February 10 2011 09:36:18 Anders Logg wrote:
> > > >> >>>>>>>>> refinements call. But I realise this fix only work for
> > > >> >>>>>>>>> SWIG 2.0. As all shared_ptr_foo are renamed to foo.
> > > >> >>>>>>>>> 
> > > >> >>>>>>>> "/home/fenicsslave/jhbuildbot/fenics/lib/python2.6/site-pac
> > > >> >>>>>>>> kage s/dolfin/mes h/refine.py", line 30, in refine
> > > >> >>>>>>>> 
> > > >> >>>>>>>> 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>.
> > > >> >>>>>>> 
> > > >> >>>>>>> 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.
> > > >> >>>>> 
> > > >> >>>>> 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.
> > > >> >>> 
> > > >> >> 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?
> > > >> 
> > > >> The sooner we can move, the better. Saves maintaining and testing
> > > >> double SWIG code.
> > > > 
> > > > 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?
> > > 

And after DOLFIN 0.9.10 I would say. I am pretty loaded for the moment...

Johan

> > > >> > them? I guess there's a small chance that Trilinos will move to
> > > >> > 2.0 any time soon.
> > > >> 
> > > >> The next PyTrilinos release will require SWIG 2.0. It should also
> > > >> fix the MPI finalisation issues.
> > > > 
> > > > ok.
> > >

---

From: Anders Logg <[email-masked]>
Date: Thu, 10 Feb 2011 19:19:37 +0100

> 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