Fixing memory leak and complexity about rebound HOT 5 CLOSED

@dtamayo

1. This is what we talked about on Friday, where I couldn't remember the reason as to why this was so complicated to begin with. The reason is that the Simulation class needs to reference itself, which is a bit tricky in python and can only be done after the class definition (that's why the _fields_ variable is set at the very end).

2. There is a performance difference between pointer(), POINTER() and byref(). It's too late right now to completely wrap my head around, but maybe this does explain the 5% performance difference that you see.
   http://stackoverflow.com/a/12281481/115102

from rebound.

I have the same memory leak in reboundxf, so thanks for letting me know. I don’t know what to say about the structure vs class. The Structure implementation seems to make more sense logically to me, though I agree it’s kind of ugly to have internal variables subscripted with ‘_’. There’s no way to overload a field’s setter function? I’ll try to look into that, as well as the pointer vs byref issue you found. That sounds quite interesting/promising.

Thanks,

Dan

On Aug 10, 2015, at 4:11 PM, Hanno Rein [email protected] wrote:

There was minor memory leak in the reb_free_simulation() function that actually did not free the simulation but only pointer therein.

I also looked at the complexity of the python Simulation class. I think this was overly complicated. Now Simulation is just a subclass of ctypes.Structure and one can access field variable directly without a setter/getter.

This raises two other issues, however:

If we want a setter on a field, e.g. for simualtion.integrator, then we need to make the fields variable private, e.g. by calling it _integrator.
The documentation for the getter/setters is gone; naturally, as the getter/setters are gone.
Development branch is on https://github.com/hannorein/rebound/tree/simsim https://github.com/hannorein/rebound/tree/simsim. I'm not totally happy with it yet and there might be an example that does not work yet (although almost all changes should be internal and not be visible from the outside).

—
Reply to this email directly or view it on GitHub #66.

from rebound.

After sleeping on in, I think the byref/pointer issue is straightforward:

• byref gives you just the address
• pointer() gives you the address as well, but is a full fledged pointer object in python (e.g. you can access the contents via sim.contents.G)
• POINTER() generates the type

from rebound.

I tried to look into the private field variables issue, and like you seem to have found that this is the best way to do things. A bit weird/frustrating, but I guess that’s OK...

Dan

On Aug 10, 2015, at 10:20 PM, Daniel Tamayo [email protected] wrote:

I have the same memory leak in reboundxf, so thanks for letting me know. I don’t know what to say about the structure vs class. The Structure implementation seems to make more sense logically to me, though I agree it’s kind of ugly to have internal variables subscripted with ‘_’. There’s no way to overload a field’s setter function? I’ll try to look into that, as well as the pointer vs byref issue you found. That sounds quite interesting/promising.

Thanks,

Dan

On Aug 10, 2015, at 4:11 PM, Hanno Rein <[email protected] mailto:[email protected]> wrote:

There was minor memory leak in the reb_free_simulation() function that actually did not free the simulation but only pointer therein.

I also looked at the complexity of the python Simulation class. I think this was overly complicated. Now Simulation is just a subclass of ctypes.Structure and one can access field variable directly without a setter/getter.

This raises two other issues, however:

If we want a setter on a field, e.g. for simualtion.integrator, then we need to make the fields variable private, e.g. by calling it _integrator.
The documentation for the getter/setters is gone; naturally, as the getter/setters are gone.
Development branch is on https://github.com/hannorein/rebound/tree/simsim https://github.com/hannorein/rebound/tree/simsim. I'm not totally happy with it yet and there might be an example that does not work yet (although almost all changes should be internal and not be visible from the outside).

—
Reply to this email directly or view it on GitHub #66.

from rebound.

Thanks. I think the only thing we need to think about is documentation. Maybe we can just put everything in the docstring for the Simulation class.

from rebound.