Request: LSR conversion ignoring 3D position

[keflavich]

The following code fails:

skycoord_withvel = coordinates.ICRS(ra=71.9*u.deg, dec=19.5*u.deg, radial_velocity=1*u.km/u.s)
skycoord_withvel.transform_to(coordinates.LSR)

with

TypeError: Position information stored on coordinate frame is insufficient to do a full-space position transformation (representation class: <class 'astropy.coordinates.representation.UnitSphericalRepresentation'>)

The workaround is:

skycoord_withvel = coordinates.ICRS(ra=skycoord.icrs.ra, dec=skycoord.icrs.dec, radial_velocity=velocity_bary,
                                    pm_ra_cosdec=0*u.deg/u.s, pm_dec=0*u.deg/u.s, distance=1*u.kpc)
lsrcoord = skycoord_withvel.transform_to(coordinates.LSR)
lsrcoord.radial_velocity

which gives the same answer for any combination of parameters except distance=0 and some weird combinations I tried (1.0 deg / s 1.0 Gpc 29895.66008725245 km / s).

This conversion should not require creating a full 3D coordinate object.

@eteq, @astrofrog, and @adrn, this is the continuation of a conversation we started at last year's astropy meeting. It might have ended up in some other issues, but I didn't find them at a glance. This certainly overlaps w/#10185, but we didn't resolve itt.

[adrn]

I believe we talked about this general situation early on when adding support for velocities in astropy.coordinates. There are other cases where assuming a distance or assuming values for proper motion components can have confusing or unexpected consequences for the user, so we decided to err on the side of "make the user be explicit about what they want to do" as opposed to special-casing certain transformations where it makes sense to fill the other component values. However, you are probably right that this transformation should be allowed because the component of interest already has velocity units.

To get into the details a bit, one barrier to easily enabling this with the current transformation machinery is that we want to be able to tell the AffineTransform that implements this transformation to ignore the position offset and only apply the velocity offset (here). Two options I see: (1) Implement something like a "NullRepresentation" that is meant to be a representation container that holds nothing (so we can attach the velocity offset as a differential object to that no-op representation), or (2) In AffineTransform, detect that the representation part of the offset is a no-op (full of 0's) and don't apply that.

cc @mhvk as well

[mhvk]

As @adrn notes, generally the result depends on knowing the distance and proper motion. For LSR, though, the radial velocity and proper motion are essentially decoupled, so indeed there is a case to do them separately. I guess the main question would be what to do with the proper motion that results (which is of course proportional to distance):

from astropy import coordinates, units as u
skycoord_withvel = coordinates.ICRS(ra=10.*u.deg, dec=10*u.deg, radial_velocity=10*u.km/u.s,
                                    pm_ra_cosdec=0*u.mas/u.yr, pm_dec=0*u.mas/u.yr, distance=1*u.kpc)
skycoord_withvel.transform_to(coordinates.LSR)
<LSR Coordinate (v_bary=(11.1, 12.24, 7.25) km / s): (ra, dec, distance) in (deg, deg, kpc)
    (10., 10., 1.)
 (pm_ra_cosdec, pm_dec, radial_velocity) in (mas / yr, mas / yr, km / s)
    (-3.41235784, 1.60666128, 7.56838273)>

I guess the options are:

1. Improve the error message.
2. Allow not having a distance and proper motion (effectively assuming distance is infinite), or no radial velocity, but then also return an instance that doesn't have those attributes.
3. Have a separate LSRNoPM class.

It also depends a little on how useful the result would be...

[keflavich]

The question I generally ask about LSR is, "What is the velocity of the barycenter wrt the LSR in that direction?" This is a meaningful question, and is more correct than assigning a position to a spectrum - for HI and CO Galactic observations in particular, there is no possible unique coordinate to define along a given line of sight, since a given spectrum contains much cloud (you might say "many clouds", but they're strictly not countable things).

The feature request is for the user to be able to ask that question. Solution (1), "Improve the error message", would really be telling the user they are wrong to ask that question, which I do not believe is true. Both options (2) and (3) seem reasonable, but perhaps the real solution is to define a bary_to_lsr_velocity function (with a better name that indicates what the sign is!!) that encapsulates solution (2) or (3).

[mhvk]

@keflavich - thanks, always helps to understand the background! A separate function or method might indeed make most sense - this does seem rather analogous to something like SkyCoord.radial_velocity_correction, which also simply asks for a velocity difference for a given direction (and location in that case).

[adrn]

this does seem rather analogous to something like SkyCoord.radial_velocity_correction

Ah, right! A 4th option would be to add a kind='lsr' option to radial_velocity_correction, which maybe also accepts an LSR frame class instance (because the solar velocity relative to the LSR is a frame attribute)

[keflavich]

This last option is what I expected to exist when I looked at radial_velocity_correction. That matches (loosely) the standards set by other software.

[mhvk]

Sounds good. Do we need a way to tell that location is already in the barycentre?

[tvwenger]

Any updates on this? From a user's perspective, it seems "wrong" to have to supply an arbitrary distance and proper motion when converting radial velocities from the heliocentric frame to the LSR frame. Requiring these values indicates to the user that they are somehow used in the conversion, which is not true and creates confusion.