search

cmateu / galstreams

Milky Way Streams Footprint Library and Toolkit for Python
BSD 3-Clause "New" or "Revised" License
46 stars 17 forks source link

Initialize stream frames with "center" instead of ra0 #25

Closed adrn closed 4 months ago

adrn commented 1 year ago

Happy new year!

I was debugging something in the GreatCircleICRSFrame and it occurred to me that I don't think providing a pole and ra0 is sufficient for uniquely setting up a coordinate frame and origin! A pole defines a great circle, and ra0 defines a great circle, so the two great circles intersect at two places, either of which could be the origin/center of the system.

An alternative would be to pass in the pole and the coordinate system origin (which I've called center for some reason). This removes the ambiguity, but then introduces the problem that a user could pass in a pole and center that are not orthogonal, and I have to decide that to do in those cases. My inclination would be to fix the center and adjust the pole slightly (along the great circle connecting pole and center) to make it orthogonal, but this has implications for galstreams: if I did that, then the track.mid_pole would no longer be the pole of the coordinate frame at track.mid_pole. The advantage, though, is that for cases where there is a progenitor system, you could set that to the frame origin and it would always be at (0, 0) in the stream frame.

One other option would be to keep the ra0 parameter and option, but to add another optional argument ra0_helper (name TBD) that lets you pass in a sky coordinate to break the ra0 degeneracy (by picking the ra0/pole intersection closest to ra0_helper). This would look like:

self.stream_frame = gc.GreatCircleICRSFrame(pole=self.mid_pole, ra0=self.mid_point.icrs.ra, ra0_helper=self.mid_point.icrs)

This has the advantage that the pole would be fixed, but means that in general you can't hard-set things (like progenitors) to be at (0,0).

Do you have any thoughts or opinions on the best way to handle this in gala?

adrn commented 1 year ago

The latest commit updates the line to be consistent with the old behavior. But this will require having people install the latest dev version of gala (until I have a new release). Will think on this...