{"action":"created","issue":{"url":"https://api.github.com/repos/hannorein/rebound/issues/65","labels_url":"https://api.github.com/repos/hannorein/rebound/issues/65/labels{/name}","comments_url":"https://api.github.com/repos/hannorein/rebound/issues/65/comments","events_url":"https://api.github.com/repos/hannorein/rebound/issues/65/events","html_url":"https://github.com/hannorein/rebound/pull/65","id":99854546,"number":65,"title":"Addxf","user":{"login":"dtamayo","id":3408023,"avatar_url":"https://avatars.githubusercontent.com/u/3408023?v=3","gravatar_id":"","url":"https://api.github.com/users/dtamayo","html_url":"https://github.com/dtamayo","followers_url":"https://api.github.com/users/dtamayo/followers","following_url":"https://api.github.com/users/dtamayo/following{/other_user}","gists_url":"https://api.github.com/users/dtamayo/gists{/gist_id}","starred_url":"https://api.github.com/users/dtamayo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dtamayo/subscriptions","organizations_url":"https://api.github.com/users/dtamayo/orgs","repos_url":"https://api.github.com/users/dtamayo/repos","events_url":"https://api.github.com/users/dtamayo/events{/privacy}","received_events_url":"https://api.github.com/users/dtamayo/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-08-09T02:30:17Z","updated_at":"2015-08-10T00:13:29Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/hannorein/rebound/pulls/65","html_url":"https://github.com/hannorein/rebound/pull/65","diff_url":"https://github.com/hannorein/rebound/pull/65.diff","patch_url":"https://github.com/hannorein/rebound/pull/65.patch"},"body":"I don't know why, but the old method for connecting the additional forces pointers doesn't work with the way I have implemented reboundxf.  But everything seems to work fine if I also cast to a C function pointer in the same way you had it for python functions (so you don't have to test what kind of function it is).  Anyway, everything seems to work when I test it (and passes travis) but wanted to make sure I wasn't missing something important.\r\n\r\nAlso, I have things set up now in reboundxf with a __del__ function that frees (in C) all the pointers in the rebxf_params.  If I understood correclty yesterday, I shouldn't free the xf_params pointer in reb_simulation in reb_free_simulation, because then I'll lose the reference to it and later be unable to free the pointers inside.  Did I get that right?\r\n\r\nThe other thing I tried for a while (this is in the rebound2 branch of my reboundxf repository), but couldn't get to work is that in addition to the decorator allowing, e.g., \r\n\r\nxf.tau_a = [0.,1e5, 1e5]\r\n\r\nfor all the particles, it would be nice to be able to do\r\n\r\nxf.tau_a[2] = 1e6\r\n\r\nbut as it stands, this just calls the getter for tau_a, and changes the 2 index of THAT list to 1e6 (but not the value in the C pointer).  Definitely don't try to look into this, but I thought I'd mention it in case you had any ideas.\r\n\r\nThanks for all your help with this!  "},"comment":{"url":"https://api.github.com/repos/hannorein/rebound/issues/comments/129263039","html_url":"https://github.com/hannorein/rebound/pull/65#issuecomment-129263039","issue_url":"https://api.github.com/repos/hannorein/rebound/issues/65","id":129263039,"user":{"login":"dtamayo","id":3408023,"avatar_url":"https://avatars.githubusercontent.com/u/3408023?v=3","gravatar_id":"","url":"https://api.github.com/users/dtamayo","html_url":"https://github.com/dtamayo","followers_url":"https://api.github.com/users/dtamayo/followers","following_url":"https://api.github.com/users/dtamayo/following{/other_user}","gists_url":"https://api.github.com/users/dtamayo/gists{/gist_id}","starred_url":"https://api.github.com/users/dtamayo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dtamayo/subscriptions","organizations_url":"https://api.github.com/users/dtamayo/orgs","repos_url":"https://api.github.com/users/dtamayo/repos","events_url":"https://api.github.com/users/dtamayo/events{/privacy}","received_events_url":"https://api.github.com/users/dtamayo/received_events","type":"User","site_admin":false},"created_at":"2015-08-10T00:13:29Z","updated_at":"2015-08-10T00:13:29Z","body":"Thanks Hanno.  I benchmarked python against C, and you were right that that implementation was about twice as slow as the C code.\r\n\r\nAfter a lot of searching online, I still can't figure out how to directly assign a C function to the additional_forces function pointer.  The issue is that when I return the C function from reboundxf, it's of type _FuncPtr, which is the default for functions from a ctypes cdll.  It then gives an error that it's expecting a CFUNCTYPE for additional_forces.  Maybe the reason it worked before is that python was not told in the reb_simulation definition that additional_forces was supposed to be a CFUNCTYPE?\r\n\r\nIn any case, I did find what I think is a good solution.  If in reboundxf I cast the function I want to a void pointer, and return that void pointer's address, I can cast that efficiently to a CFUNCTYPE.  Despite some effort, I still don't know why it's faster, but it's a solution I found online.  It's also really nice, because it means I don't have to cast it from within the python side of reboundxf--this would require that module to know where rebound is to import it and have the reb_simulation definition (since it's the passed parameter to additional_forces).  So I think that's a huge plus.  \r\n\r\nThe code in reboundxf looks like this:\r\ndef modify_elements():\r\n     func_address = cast(clibreboundxf.rebxf_modify_elements, c_void_p).value\r\n     return func_address\r\n\r\nI benchmarked this against the C code and for a particular simulation got\r\n12.4 s vs 10.9 over 1e5 orbits, and\r\n117.5 vs 110.6 over 1e6 orbits,\r\n\r\nso it seems it asymptotes to being about 5% slower than the C code, which seems quite good.  Let me know what you think."}}