Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Where should Redshift/RV be stored on Spectrum objects, and how to transform between different forms - RV(opt)/RV(rad) #258

Closed
SaOgaz opened this issue Jul 23, 2018 · 12 comments

Comments

@SaOgaz
Copy link
Contributor

SaOgaz commented Jul 23, 2018

original card by @keflavich

@stscijgbot
Copy link

This ticket is now being tracked at JDAT-7

@ibusko
Copy link
Contributor

ibusko commented Jul 23, 2019

To implement this without solving the idiom issue at #455 probably means we should do it in a top-level function in the spectrum1d.pymodule. We can later move it to wherever we decide the redshift/rv info should be stored.
The conversion will certainly be using the already existing Spectrum1D parameters rest_value and velocity_convention. What are these exactly? Is rest_value a scalar or a 1-D array? Is velocity_convention just a way to tell the difference between frequency and wavelength definitions? I bet there is more than that. What doppler_relativistic means in this context?

@nmearl
Copy link
Contributor

nmearl commented Jul 23, 2019

rest_value is the wavelength used in the velocity conversions (e.g. wavelength -> velocity conversions will result in dramatically different dispersions depending on which wavelength is considered the rest wavelength).

Is velocity_convention just a way to tell the difference between frequency and wavelength definitions?

No, velocity_convention defines which of the three velocity conventions to use in a dispersion conversion (radio, optical, relativistic). Each uses a different formula.

@ibusko
Copy link
Contributor

ibusko commented Jul 23, 2019

Aren't radio and optical essentially the same conversion? The differences in between the formulae of these two have to do with spectral axis units, but for the non-relativistic approximation, the formulae perform the same operation. Or am I missing something?

Maybe we need a clarification on the purpose of this translation. Is it meant to generate a velocity axis in velocity units, against which the spectrum data can be paired? Is it meant to generate a rest spectral axis in wavelength/frequency units? Or something else?

This discussion would probably benefit from work done under #455

@nmearl
Copy link
Contributor

nmearl commented Jul 23, 2019

Aren't radio and optical essentially the same conversion?

No. The differences in the formulas should be obvious below:

Radio:
rad_vel

Optical:
opt_vel

Relativistic:
rel_vel

The differences in between the formulae of these two have to do with spectral axis units

I don't know what you mean by this; all of the formulas expect a dispersion in frequency (or wavelength) space and produce a spectral axis in velocity space.

The purpose of the velocity convention is how to convert the spectral axis from frequency or wavelength space to one in velocity space (or the reverse). Depending on your needs and the characteristics of the observation, you'll use a different convention. E.g. Optical velocities are commonly used for (Helio/Barycentric) extragalactic observations; (LSR) radio velocities are typical for Galactic observations.

@ibusko
Copy link
Contributor

ibusko commented Jul 23, 2019

OK, thanks for the references, it's clearer now. Do you guys agree that a top-level function is the way to go for now? I envision a function that has as input a (properly initialized) Spectrum1D object, and returns the velocity vector, a numpy Quantity array.

Are there use cases where one would benefit with the reverse translation as well (velocity->w/f/e)?

@nmearl
Copy link
Contributor

nmearl commented Jul 23, 2019

I'm not sure what you mean. Specutils already supports converting to velocity space by simply calling my_spectrum.velocity. This internally uses the defined velocity convention. The rest value is already stored on the Spectrum1D object, and velocity convention can be changed by calling the with_velocity_convention method. However, the reverse is currently not implemented: i.e. you can't initialize a spectrum object with a velocity dispersion and have my_spectrum.wavelength/my_spectrum.frequency convert from it.

The only thing specutils does not handle currently is redshifting.

@ibusko
Copy link
Contributor

ibusko commented Jul 23, 2019

Ah, OK, I was confused because I was looking at the code in spectrum1d.py, while the relevant machinery already exists in spectrum_mixin.py

@ibusko
Copy link
Contributor

ibusko commented Jul 26, 2019

I assume from the title that this is supposed to handle both redshift (a-dimensional), and radial velocity (quantity with velocity units). Any ideas on how to implement the distinction in a user friendly way?
I though initially that a single redshift_rv keyword parameter in the Spectrum1D constructor would suffice. The user would provide either a float or a Quantity to tell apart one from the other. But it has the drawback of being error-prone, in the sense that users would often forget to create a proper Quantity instance when intending to use radial velocity, and that would result in weird results (and not an error or exception).

@eteq
Copy link
Member

eteq commented Aug 9, 2019

Note: I think the direction that things are going to go for this is to have the spectral_axis be a SpectralCoord object, which is something we've realized needs to live in astropy because it's also needed in WCS's. There already are rules for doing this translation that apply in the wcs context, and we want to be sure we use the same machinery in specutils as we do elsewhere.

That said, we can still keep this issue open for the specutils side of things. Once the SpectralCoord has this implemented we might decide we want to add some conveniences to make it a bit easier on the user side like what @ibusko is suggesting. (E.g. Spectrum1D.redshift_rv could be a property that defers to Spectrum1D.spectral_axis.redshift or similar) and this issue would still track that.

@eteq eteq changed the title Redshift/RV (opt)/RV(rad): translation between them, and *maybe* storage on spectrum objects Where should Redshift/RV be stored on Spectrum objects, and how to transform between different forms - RV(opt)/RV(rad) Feb 10, 2020
@nmearl
Copy link
Contributor

nmearl commented May 6, 2020

SpectralCoord has been implemented with appropriate handling for redshifting and radial velocities.

@nmearl nmearl closed this as completed May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants