Yes I understand, and agree that it would be better if the memory management
was a bit easier
I've also found that there is a fairly blurry line between 'pythonicness'
and 'poor/ugly API'. Having written about 2000+ lines of code, i've written
a lot of wrapper classes.
Many of these wrappers i'd wager are nearly identical to those any other
python developer writes as soon as they deal with openCascade. for example
generators to convert topexp iteration to a for/in loop, edge wrappers to
return points convieniently, convienience methods to created edges and
curves ( I have a function nearly the same as yours in my code btw , case
in point ).
I guess what i'm saying is that, though garbage collection helps, its also
true that your MakeEdge method is somewhat compensating for a klunky api
that doesnt just return Shape() and throw an exception instead of doing
Perform() then IsDone() and such. Its clear that the solution to this is to
let people contribute helper and wrappers over time so that we all benefit.
I think what we need from Thomas/Jelle is a place that these can be checked
in and then apidoc is autogenerated. I'd be more than happy to commit some
of my utilty/wrapper code, but i'd want to know where i can look to see the
apidoc for the other such code is before I add it...
On Wed, Mar 17, 2010 at 1:09 PM, Frank Conradie <fr...@qfin.net> wrote:
> Hi Dave
>
> Thanks for the suggestion - closures would certainly help. However, I am
> currently less interested in ways to work *around* the "un-pythonic"
> behavior of the wrapper, but would rather like to discuss the technicalities
> and politics(!) of making the wrapper more "pythonic". Here is what I would
> ideally like to do:
>
> * Clear up exactly what "un-pythonic" behavior I have run into while using
> the library for a fairly complicated algorithm (two modules, 3000+ lines of
> code).
> * Discuss whether it is *technically* possible to change any of this
> behavior to be more "pythonic"
> * If it is indeed *technically* possible, discuss whether it is feasible
> and worthwhile to actually implement
>
> Here is a summary of the current "unexpected" behavior as I see it:
>
> (1). When using OCC objects in Python, an unsuspecting programmer would
> expect them to behave like Python objects, i.e. use Python ref counting and
> garbage collection behavior. E.g. a BRepBuilderAPI object has a TopoDS_Shape
> member, so this "myShape" has a refcount of 1 initially, and if it is
> retrieved via the Shape() method and put into, say, a list, its refcount
> must be 2, etc. So even if the BRepBuilderAPI object goes out of scope, its
> internal shape is expected to still have a refcount of 1 and "survive" a
> garbage collector purge. However, this refcounting obviously does not happen
> on the C++ side, so the BRepBuilderAPI object would have to be kept "alive"
> as long as its internal shape is needed.
>
> (2). Getting the "internal" shape from a BRepBuilderAPI, BRepPrimAPI,
> BRepAlgoAPI, etc. object:
> * C++: calling Shape() always returns the *same* myShape instance
> * Python: calling Shape() returns a *new* SWIG wrapper object every
> time, each with the same __hash__ value so they "look" like the same
> instance (at least to sets and dicts)
> * The same unexpected behavior occurs when "exploring" a shape, i.e.
> new SWIG wrapper objects are instantiated for every returned shape
> * The fact that different SWIG wrapper objects hash to the same value
> at least allows us to identify unique shapes when put in sets and dicts, but
> it confuses the GarbageCollector, as the same C++ object can be "collected"
> multiple times, once for each SWIG object wrapping it
>
> Now, I can live with point (1), as I can see how this would be near
> impossible to implement to work like it would in Python. However, I really
> would like to discuss whether point (2) can be addressed, so that there is
> always a 1-to-1 correspondence between a C++ object and its SWIG wrapper
> object. The SWIG wrapper already generates a single unique __hash__ value
> for a single C++ object, so would it not be feasible to keep a map of C++
> object --> hash value --> SWIG wrapper object?
>
> In pseudo-code, when the wrapper is asked for an OCC object:
>
> h = hash(occobject)
> if h in occ2swig_map:
> swig_wrapper = occ2swig_map[h]
> else:
> swig_wrapper = new SwigWrapper(occobject)
> occ2swig_map[h] = swig_wrapper
> return swig_wrapper
>
> I have never used SWIG myself, so excuse any ignorance on my side (I will
> try and spend some time looking deeper into SWIG over the next few days).
>
> Hopefully we can have a fruitful and open discussion about this. I can
> certainly work with the library as-is, and am indeed doing so successfully
> at the moment. I just find that it is no fun having to deal with the very
> kinds of C++ issues I am trying to avoid by using Python in the 1st place
> ;-)
>
> Cheers,
> Frank
>
>
>
>
> On 17/03/2010 4:22 AM, Dave Cowden wrote:
>
> Hi, Frank:
>
> I am not sure, but I think you could implement your utility function
> using a python closure. This would allow the original object to remain in
> scope until the nested function has completed. so for example:
>
> To do closures in python, you have to use lamda, which creates a new
> function having access to the outer scope, even after the function returns.
> So
>
> def MakeEdge(p1, p2):
> gp1,gp2 = [gp_Pnt(x,y,z) for x,y,z in (p1,p2)]
> mkedge = BRepBuilderAPI_MakeEdge(gp1, gp2)
> return mkedge.Shape()
>
> Becomes:
>
> def MakeEdge(p1,p2):
> gp1,gp2 = [gp_Pnt(x,y,z) for x,y,z in (p1,p2)]
> mkedge = BRepBuilderAPI_MakeEdge(gp1, gp2)
> return lambda e: mkedge.Shape();
>
> now you can do
> me = MakeEdge(p1,p2);
>
> me.Shape() //return the generated shape
> me.Shape() //return the generated shap again. It will be the same object
> because the underlying brepbuilder is still in a closure
>
> Now if you kill me, the closure will be discarded.
>
> Dont know if this helps any or not.
>
> On Tue, Mar 16, 2010 at 7:53 PM, Frank Conradie <fr...@qfin.net> wrote:
>
>> Hi Thomas
>>
>> After much investigation today, I have now added a flag to
>> GarbageCollecter to disable it for our use, as it really does not work as
>> intended for relatively complex code (I now manually track which objects I
>> create and need to call _kill_pointed for). There are two main issues that
>> we run into with the current GarbageCollector:
>>
>> 1. As already discussed, classes like the BRepPrimAPI and BRepAlgoAPI have
>> an internal "myShape" data member that gets destroyed when the parent
>> object's _kill_pointed method is called, causing all kinds of issues. As an
>> example, it makes utility functions like the one below unusable, as the
>> local BRepPrimAPI object goes out of scope and is thus collected by
>> GarbageCollector for purging, even though the caller of the function may
>> want to use the resulting shape for some time to come, after the parent is
>> "purged":
>>
>> def MakeEdge(p1, p2):
>> gp1,gp2 = [gp_Pnt(x,y,z) for x,y,z in (p1,p2)]
>> mkedge = BRepBuilderAPI_MakeEdge(gp1, gp2)
>> return mkedge.Shape()
>>
>> 2. Every time a C++ OCC object is retrieved from Python in any way, a
>> brand new SWIG wrapper object is instantiated for it, very easily/quickly
>> leading to multiple wrapper objects on the Python side for the same
>> underlying C++ object (I see this especially when using OCC shape containers
>> repeatedly). When any one of these wrapper objects goes out of scope and is
>> collected and purged, the underlying C++ object is destroyed (via
>> _kill_pointed), even though there may well be other Python wrapper objects
>> still needing access to it.
>> I guess the ideal solution to this issue would be to keep a mapping of
>> C++ object -> SWIG wrapper object on the C++ side, so that there is always a
>> 1-to-1 correspondence between the two, but I'm not sure how plausible this
>> really is from an implementation point-of-view. As an example of what I
>> mean, consider this: every time you call "mkedge.Shape()" in my example
>> above, you get a new/different Python object, wrapping the same underlying
>> myShape instance, which is totally unexpected and unintuitive from a Python
>> perspective. A "pure" Python programmer would expect to get the exact same
>> Python object back every time, and not different wrapper objects with just
>> an "artificial" identical hash value.
>>
>> I realize that this is a very difficult issue, and that as users of the
>> PythonOCC wrapper library we will need to take responsibility for memory
>> management issues, but I can see lots of confusion for Python programmers
>> using the library and expecting "Python-like" behaviour out of the box.
>>
>> I wish I new more about the SWIG wrapper object instantiation process so
>> that I could make more concrete suggestions, instead of just complaining
>> ;-) What is your gut-feeling about this Thomas - do you think these two
>> issues can be addressed in the wrapper, or is the current way the only
>> viable implementation? If you can give me a short description of the SWIG
>> wrapper instantiation process, I may be able to delve into it myself.
>>
>> Thanks for your help with this thus far,
>> Frank
>>
>>
>>
>>
>> On 16/03/2010 9:04 AM, Thomas Paviot wrote:
>>
>> Hi Frank,
>>
>> I also suggest you use the push_context() and pop_context() methods at
>> the beginning and the end of your algorithm. garbage.push_context() creates
>> a new collector. Objects collected will go to this new list. Then, at the
>> end of your algorithm, just call .smart_purge() and then pop_context().
>> Otherwise, when calling the smart_purge() method, you'll erase all other
>> objects that might have been stored in the gc thus causing strange things.
>>
>> def my_high_memory_consuming_algorithm():
>> GarbageCollector.garbage.push_context()
>>
>> ... do whatever you want ...
>>
>> GarbageCollector.garbage.smart_purge() #only the objects created
>> since the call to push_context are erased. Use MMGT_OPT=0 to make sure
>> freeing #memory.
>> GarbageCollector.pop_context()
>> return data
>>
>> There's also a garbage.prevent_object(object) method, that will take
>> care to not delete an object that it's important to keep reference.
>>
>> These three functions are quite new (but validated with a unittest). I'm
>> curious to have your feedback!
>>
>> Regards,
>>
>> Thomas
>>
>> These two functions are quite new,
>> 2010/3/16 Frank Conradie <fr...@qfin.net>
>>
>>> Hi Thomas
>>>
>>> Well, I'm glad it's not a bug then! I can think of a few ways to handle
>>> this situation in my algorithm, so it should not be a problem. I'm just
>>> wondering how many other classes behave in this "unexpected" way :O
>>>
>>> Cheers,
>>> Frank
>>>
>>>
>>> On 15/03/2010 10:26 PM, Thomas Paviot wrote:
>>>
>>> Hi Frank,
>>>
>>> This behaviour does not come from SWIG but from OpenCASCADE itself. Let
>>> me explain you in a few words what happens here.
>>>
>>> The BRepPrimAPI_MakeBox class inherits from the
>>> BRepBuilderAPI_MakeShape one. This class provides the Shape() method you can
>>> find in BRepPrimAPI_Make* basic geometry constructors. If you have a look at
>>> the Shape() method in the file BRepBuilderAPI_MakeShape.cxx, you will read:
>>>
>>> """
>>>
>>> //=======================================================================
>>> //function : Shape
>>> //purpose :
>>> //=======================================================================
>>>
>>> const TopoDS_Shape& BRepBuilderAPI_MakeShape::Shape() const
>>> {
>>> if (!IsDone()) {
>>> // the following is const cast away
>>> ((BRepBuilderAPI_MakeShape*) (void*) this)->Build();
>>> Check();
>>> }
>>> return myShape;
>>> }
>>> """
>>>
>>> The Shape() method then returns a reference to the protected attribute
>>> 'myShape' of the class BRepBuilderAPI_MakeBox. In other words, if you delete
>>> the BRepPrimAPI_MakeBox instance, you loose the reference to the shape.
>>>
>>> For instance:
>>>
>>> >>> from OCC.BRepPrimAPI_MakeBox import *
>>> >>> from OCC.TopoDS import *
>>> >>> box = BRepPrimAPI_MakeBox(10,10,10)
>>> >>> shp = box.Shape()
>>> >>> print shp.IsNull()
>>> 0
>>> >>> del box #object is garbage collected, not deleted yet
>>> >>> print shp.IsNull()
>>> 0
>>> >>> GarbageCollector.garbage.smart_purge() # all objects are deleted,
>>> loose the reference to myShape attribute
>>> >>> print shp.IsNull()
>>> 1
>>>
>>> Then force objects deletion only if you're sure you don't need the
>>> shapes anymore. I don't know what you're trying to do exactly, so it's
>>> difficult to suggest any best practice.
>>>
>>> Regards,
>>>
>>> Thomas
>>>
>>> 2010/3/15 Frank Conradie <fr...@qfin.net>
>>>
>>>> Hi Thomas
>>>>
>>>> I am getting crash behaviour in the interim 0.5 PythonOCC release,
>>>> related to the new GarbageCollector implementation. The code below
>>>> crashes on my PC:
>>>>
>>>> ----------------------------------------
>>>> from OCC.gp import *
>>>> from OCC.BRepPrimAPI import *
>>>> from OCC.BRepBuilderAPI import *
>>>> from OCC.TopExp import *
>>>> from OCC.TopoDS import *
>>>>
>>>> o = 0.0
>>>> w = 0.1
>>>> fp1 = (o,o,o)
>>>> fp2 = (w,w,w)
>>>>
>>>> mkbox = BRepPrimAPI_MakeBox(gp_Pnt(fp1[0],fp1[1],fp1[2]),
>>>> gp_Pnt(fp2[0],fp2[1],fp2[2]))
>>>> s = mkbox.Shape()
>>>> del mkbox
>>>>
>>>> print s.ShapeType()
>>>> GarbageCollector.garbage.smart_purge()
>>>> # The next line crashes my system
>>>> print s.ShapeType()
>>>> ----------------------------------------
>>>>
>>>> It is almost as if calling _kill_pointed for the mkbox causes shape "s"
>>>> to become "corrupted" in some way. I dug through the SWIG code a bit,
>>>> but cannot see anything obviously wrong, so I was hoping you could shed
>>>> some light on the issue.
>>>>
>>>> By the way, I did get the SVN compiled myself as well, and get the same
>>>> crash with my compiled lib as well as the one you made available on your
>>>> website.
>>>>
>>>> Thanks,
>>>> Frank
>>>>
>>>>
>>>> _______________________________________________
>>>> Pythonocc-users mailing list
>>>> Pythonocc-users@gna.org
>>>> https://mail.gna.org/listinfo/pythonocc-users
>>>>
>>>
>>>
>>> _______________________________________________
>>> Pythonocc-users mailing
>>> listpythonocc-us...@gna.orghttps://mail.gna.org/listinfo/pythonocc-users
>>>
>>>
>>> _______________________________________________
>>> Pythonocc-users mailing list
>>> Pythonocc-users@gna.org
>>> https://mail.gna.org/listinfo/pythonocc-users
>>>
>>>
>>
>> _______________________________________________
>> Pythonocc-users mailing
>> listpythonocc-us...@gna.orghttps://mail.gna.org/listinfo/pythonocc-users
>>
>>
>> _______________________________________________
>> Pythonocc-users mailing list
>> Pythonocc-users@gna.org
>> https://mail.gna.org/listinfo/pythonocc-users
>>
>>
>
> _______________________________________________
> Pythonocc-users mailing
> listpythonocc-us...@gna.orghttps://mail.gna.org/listinfo/pythonocc-users
>
>
> _______________________________________________
> Pythonocc-users mailing list
> Pythonocc-users@gna.org
> https://mail.gna.org/listinfo/pythonocc-users
>
>
_______________________________________________
Pythonocc-users mailing list
Pythonocc-users@gna.org
https://mail.gna.org/listinfo/pythonocc-users
