On Thu, Feb 10, 2011 at 09:16:20AM +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); > > > > 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.
That's an easy fix. I'll wait for the merge with Marie's branch and add it then. I really don't see what the big issue is. If there are some details of the implementation which are not 100% correct (like missing duplicate versions taking references or shared_ptr) we will fix those details. It's no reason for throwing out the implementation and starting from scratch. The implementation itself is in pretty good shape and solves a complex problem in a relatively small amount of code. -- Anders _______________________________________________ Mailing list: https://launchpad.net/~dolfin Post to : [email protected] Unsubscribe : https://launchpad.net/~dolfin More help : https://help.launchpad.net/ListHelp
