hannorein / rebound

💫 An open-source multi-purpose N-body code.
https://rebound.readthedocs.io/
GNU General Public License v3.0
836 stars 218 forks source link

Rotations #638

Closed dtamayo closed 1 year ago

dtamayo commented 1 year ago

Hi Hanno, @tigerchenlu98, @shadden

Playing around with tides_spin (and debugging my own issues!) I think we do need to add some functionality for doing rotations in order for people to be able to use it easily. Obviously none of this is specific to spins, so I think it makes sense for these general functions to be in REBOUND. Not looking to merge now, but wanted to give you working code so you could try it out.

Probably the best way to get a feel for what might be a good API is to try out different things one might to track/plot in the examples. I love all the great examples Tiger's put together--I thought I'd add an even simpler example to more slowly go through some of the issues someone new would need to think through carefully. I think the issues with what might be the best API might be best resolved by looking through that example and how I'm calculating things. You can find it on the newpr91 branch of REBOUNDx (I've also sent Tiger a pull request to add to the main pull request).

https://github.com/dtamayo/reboundx/blob/newpr91/ipython_examples/SpinsIntro.ipynb

Here are my main questions:

The main question I have is whether we want properties and methods to return vec3ds or lists. For example I've added a hvec and evec attribute to reb_orbit (both as reb_vec3ds) so we can easily get the orbit normal and eccentricity vectors. On the one hand it's nice if these are returned as reb_vec3ds if we want to subsequently want to rotate them using the built-in methods. But we're typically also trying to store outputs from calls to sim.integrate in lists. So we want to say something like h[i] = ps[1].hvec, but this doesn't work if hvec returns a reb_vec3d.

sim.rotate_simulation(normalvec=rebx.calculate_total_angular_momentum())
rebx.rotate_params(normalvec=rebx.calculate_total_angular_momentum())

you would get the wrong behavior, since you'd have rotated the orbits in the first call, and when you called rebx.calculate_total_angular_momentum() the second time, you would get a different answer (with rotated orbits but unrotated spins). Not sure what the best approach is there.

dtamayo commented 1 year ago

Definitely! I think the clearest thing to say in the documentation is just what you said: rot * [1,0,0] maps [1,0,0] into the orbital plane, i.e., it gives a vector pointing toward pericenter in the reference xyz coordinates.

hannorein commented 1 year ago

Ok. Making that change and updating the tests...

Amazingly long discussion for what can literally be written in 4 lines of code 😉:

struct reb_rotation P1 = reb_rotation_init_angle_axis(omega, (struct reb_vec3d){.z=1});
struct reb_rotation P2 = reb_rotation_init_angle_axis(inc, (struct reb_vec3d){.x=1});
struct reb_rotation P3 = reb_rotation_init_angle_axis(Omega, (struct reb_vec3d){.z=1});
return reb_rotation_mul(P3, reb_rotation_mul(P2, P1));
dtamayo commented 1 year ago

Haha, just means it's well written code, and that this is confusing! I personally don't want to figure this out again :)

dtamayo commented 1 year ago

Oops, I realized that I didn't update to_new_axes to make newx perpendicular. Let me know when you're done and I'll add that!

hannorein commented 1 year ago

Yes, please add that!

dtamayo commented 1 year ago

(oops, fixing now)

hannorein commented 1 year ago

Amazing, I think this means we're ready to merge!

dtamayo commented 1 year ago

Maybe you already fixed it, but I pushed the syntax fix and updated the unit test for orbital to match.

Thank you!!

hannorein commented 1 year ago

Yes. Thank you!