dolfin team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

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

It is different, because elsewhere we provide an 'advanced' interface
that takes shared pointers.

Garth

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