dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Thu, Feb 10, 2011 at 09:25:55AM +0000, Garth N. Wells wrote:
>
>
> On 10/02/11 09:13, Anders Logg wrote:
> > 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);
> >
>
> This doesn't work. I get a compile error.

That's strange.

mesh = refine(mesh);

is used in the the standard demo in DOLFIN (demo/undocumented/refinement)
which builds find both on my laptop and on the buildbots.

I expect you are doing something like this:

  UnitSquare mesh(2, 2);
  mesh = refine(mesh);

That doesn't work because you assign from a Mesh to a UnitSquare. That
has never worked. If you look at the demo it does something like this:

  UnitSquare unit_square(2, 2);
  Mesh mesh(unit_square);
  mesh = refine(mesh);

That's not a result of the hierarchy. It's always been that way.

--
Anders



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