dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Thu, Feb 10, 2011 at 08:44:21AM +0000, Garth N. Wells wrote:
>
>
> On 10/02/11 07:19, Anders Logg wrote:
> > B1;2600;0cOn Wed, Feb 09, 2011 at 11:03:28PM +0000, Garth N. Wells wrote:
> >>
> >>
> >> On 09/02/11 22:43, Anders Logg wrote:
> >>> On Wed, Feb 09, 2011 at 10:11:40AM -0800, Johan Hake wrote:
> >>>> Hello!
> >>>>
> >>>> I am pretty sure the reason the Macbot still complains (mesh unit test) is
> >>>> that refine is broken for SWIG 2.0.
> >>>>
> >>>> I think it is some premature destruction of a refined mesh. I would suggest we
> >>>> implement a full shared_ptr version of the interface to get around this
> >>>> problem. I have no clue of why it works for SWIG 1.3.40. Probably because a
> >>>> faulty implementation.
> >>>>
> >>>> I also suggest more developers upgrade to SWIG 2.0.1 and maybe one of the
> >>>> linux build bots two? If it is only the Macbot that uses SWIG 2.0 it is easily
> >>>> to think it is some Mac specific error.
> >>>>
> >>>> Johan
> >>>
> >>> I plan to merge with main tomorrow if my buildbot is green. Then Marie
> >>> also needs to merge (we have both touched refine.h/cpp). Then we can
> >>> sort out the shared_ptrs.
> >>>
> >>
> >> I really don't get the approach to hierarchies. Mesh refinement was
> >> simple, and now something simple has become complex (with bugs that are
> >> hard to track down because of the introduced complexity).
> >
> > I really don't understand what the problem is. Mesh refinement is just
> > as simple as it used to be, the Mesh class itself is (almost) as
> > simple as it used to be (one added inheritance), and the Hierarchical
> > base class is also a very simple class (just stores two pointers for
> > child and parent). All the complexity is in refine.cpp, which only
> > kicks in if you do something like refine(variational_problem) or
> > refine(function_space).
> >
> >> This hierarchy business looks viral - now I get Swig warnings for
> >> DirichleBC.
> >>
> >> I would much rather keep basic classes simple, and have hierarchical
> >> containers that can keep track of parent/child relationships. The
> >> present approach seems to take a narrow/immediate view on the issue.
> >
> > I think that would lead to more complex code. We would need to store
> > quite a few different relationship trees separately from the objects.
> >
> > The problems we see now are related to SWIG and correct use of
> > shared_ptrs. We've had quite a few SWIG/shared_ptr problems in the
> > past and figured out how to fix them. I'm sure we will handle this
> > too.
> >
>
> I would like to remove the mesh refine functions, or re-write them. My
> objections to the current implementation are:
>
>  - It forces a mesh hierarchy on users, even if this is not desired

It doesn't. Normal users will do

  mesh = refine(mesh);

and then no hierarchy is stored (in C++). It would be very easy to add
a parameter "store_hierarchy" which defaults to false if that is
desired.

>  - It is not const-correct.

I believe it is const-correct in Marie's branch.

>  - The 'parent' can go out of scope, leaving the 'child' with a dangling
> pointer.

It shouldn't go out of scope since we use shared ptrs. It's the same
issue as everywhere else where we pass by reference and use
reference_to_no_delete_pointer. It's not special for the refinement
functions.

--
Anders


> We should be very particular about const-correctness throughout (i.e.
> not use const_cast unless absolutely necessary) because it makes
> multi-threading *much* easier to develop.
>
> Garth
>