dtamayo / reboundx

A library for adding additional forces to the REBOUND N-body integration package
GNU General Public License v3.0
80 stars 60 forks source link

Adding gas dynamical force used in Generozov&Perets arxiv:2212.11301 #104

Closed alekseygenerozov closed 1 year ago

alekseygenerozov commented 1 year ago

Hello,

I created some code to calculate the gas dynamical friction force from a gas disk, and would like to make it public. I tried to follow the reboundx instructions for adding a new effect as closely as possible but for the life of me I could not get the documentation to appear in the correct format.

Thank you,

Aleksey

dtamayo commented 1 year ago

Hi Aleksey, this looks great, thanks! I can help with the documentation, and will work on it early next week

dtamayo commented 1 year ago

Hi Aleksey,

Is there any way to write this code without the GNU scientific library? Incorporating that as a dependency adds quite a bit of complexity and failure points trying to get things to work across a range of systems and packaging it on PyPI. Looping in @hannorein but I think this is something we would strongly want to avoid to avoid a wide range of future headaches.

hannorein commented 1 year ago

I agree, I would try hard to avoid using such dependencies. I just had a quick look and couldn't figure it out. What is GSL used for here?

alekseygenerozov commented 1 year ago

Oops. The gsl library is not actually used for this effect, and I did not mean to include it here. I think it must be left over from another effect I experimented. The lines with

include <gsl/gsl_sf_exp.h>

include <gsl/gsl_sf_erf.h>

in gas_df.c can be deleted. This is the only place gsl appears, right?

dtamayo commented 1 year ago

Great! I removed those lines, fixed the documentation and sent you a pull request (which should show up here once you merge it). I can never get the documentation right either unless I work from one of the other effects' headers.

People might find it more usable if in the .ipynb you wrote out your equations that you give in the effect parameter descriptions, since you're able to write it all out nicely in Latex (or at least link back to the documentation or your paper).

Up to you--i'll wait to hear back and I think it's ready to merge!

dtamayo commented 1 year ago

Let me know if you'd like to add some context for the ipynb, or just want me to merge this. Is it OK with you if I change the effect name to gas_dynamical_friction instead of gas_df?

alekseygenerozov commented 1 year ago

Hi Dan,

Sorry for the late response. I just added some documentation to the ipynb example, and pushed the changes to the fork. Should I submit a new pull request? Please feel free to rename the effect.

dtamayo commented 1 year ago

This looks amazing! Thanks Aleksey, I think a lot of people will find this very useful

dtamayo commented 1 year ago

Everything's up now

hannorein commented 1 year ago

I agree this should be very useful! (Although the best prescription of these forces is of course Rein 2012 😉)