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

Decide on idiom for recording redshift/velocity of spectra #455

Closed
eteq opened this issue Mar 4, 2019 · 7 comments
Closed

Decide on idiom for recording redshift/velocity of spectra #455

eteq opened this issue Mar 4, 2019 · 7 comments

Comments

@eteq
Copy link
Member

eteq commented Mar 4, 2019

This issue actually touches on several topics and is more of a roll-up of some related issues. But we do need to answer this to move forward. The core question is how we want to store the concept of "the redshift of this spectrum".

First, the specific related issues:

  • Implement SpectralCoord class #422 - proposes creating a SpectralCoord (one minor awkwardness: requires changes in gwcs at the same time as specutils). This isn't immediately obvious related, until you realize it would have a rest attribute, which is implicitly specifying a "redshift". So wherever the redshift is specified has to be synced up with that somehow. I also thing it's not the way most astronomers are used to thinking, so we have to either spend a lot of effort training them in or "hide it" somehow for use cases that don't require its full power.
  • Implement frame transformations #413 - roughly speaking, this is that it should be possible for SpectralCoords to transform themselves. But it's mixed together with the question of whether that should be done on the spectrum vs separately
  • Where should Redshift/RV be stored on Spectrum objects, and how to transform between different forms - RV(opt)/RV(rad) #258 - this is basically "we should be able to do the radio<->optical conversion" (which the above two would address) but also whether or not this should be stateful on the spectrum.
  • Heliocentric correction calculation #230 - If we interpret this issue as "be able to correct the spectrum" it's also tied in, in that it's basically "do the correction in a very precise manner when needed". But an important point that reveals I think is that the user often needs to have a lot of control over how the velocity corrections are applied: some of the cutting-edge science depends on very careful velocity work, so we shouldn't hide it behind layers of classes that the average astronomer might not bother with/want to understand.

Now for the questions (which are sort of questions about both Spectrum1D and SpectralCoord):

  1. Should the redshift be stored on the spectrum objects themselves?
  2. How do we handle redshifts for the array Spectrum1D cases? (potentially problematic mainly for the SpectralCoord contains the redshift b/c then the SpectralCoord is not separable from the spectrum)
  3. How do we handle redshifts for the SpectrumCollection case? (probably easier for the SpectralCoord case, but should be not terribly inconsistent with 2 above)
  4. Depending on the answers to the above: how do we go about "de-redshifting" (which is generally just re-mapping the spectral_coord, rather than the flux, although a valid option is "don't do this")

I have some ideas, but want to allow room for discussion without spoiling things...

cc @keflavich @crawfordsm @hcferguson @astrofrog @nden

@eteq
Copy link
Member Author

eteq commented Mar 4, 2019

A data point #1 from an astronomer I surveyed (not on github) who does resolved (faint, relatively low-res) stellar spectroscopy: to him it was natural to keep the redshift separate from the spectrum object, and apply redshift-related operations. (As opposed to having the state of the redshift on the spectrum itself)

@keflavich
Copy link
Contributor

I don't have a strong opinion here since I work at z=0. As an outsider, it would be helpful to see additional explicit use cases laid out.

@eteq
Copy link
Member Author

eteq commented Mar 4, 2019

@keflavich - A few example use cases off the top of my head (others are welcome to chime in with more!):

  1. A Spectrum1D is of a galaxy. The user recognizes the line pattern and by eye and determines the redshift to be z=6.1 by centroiding on a single recognizable line. They then want to ask the question "what's the flux in line X, which is at rest wavelength lambda0". They want to then make that measurement, but just specify the rest wavelength (and maybe a line width). How do they connect the redshift information to the line and/or adjust the spectrum itself? The options I see there are: 1. attach the redshift to the spectrum, and have a method "de-redshift" that yields a rest-wavelength spectrum that then the flux is measured on 2. attach the redshift to the spectrum, but have the SpectralRegion have a flag that indicates it's the rest wavelength (perhaps just a SpectralCoord?), which signals to the flux-measuring function that it should use the redshift on the spectrum to shift the region to the observed frame 3. have the flux-measuring function accept a redshift which tells it to do the shifting appropriately for a SpectralRegion which is implicitly taken to be "rest wavelength".

  2. A collection of fiber spectra that are all rectified to the same observed wavelength, where many of the spectra are stars with a measurable radial velocity. This is stored as an array Spectrum1D. The users runs some automated routine to determine all the velocities, and then wants to both create a table of all the RVs. Should they store it in the Spectrum1D or should the output of the measurement function give its own set of redshift/RVs that are independent of the Spectrum1D? What if the user wants to pass that collection (along with its redshifts) onto some other function to do more processing?

  3. Same spectra as case 2 but the user wants to measure some lines (a la case 1). All the same questions as case 1 apply, but with the added wrinkle of: if the redshifts are stored in the SpectralCoord of the Spectrum1D, how do we account for the fact that the redshifts are a vector associated with the Spectrum1D object even though the SpectralCoord is only 1D (i.e., doesn't have a dimension that matches the non-pixel dimension of the flux)

@eteq
Copy link
Member Author

eteq commented Aug 9, 2019

An item that has just come up in this direction: the spectral coord from #422 we've realized probably needs to (indirectly) encode the redshift in it. In principle, the observer->target line-of-sight velocity is the velocity that encodes the redshift. So we should not be duplicating that information in two places.

Now it's probably quite natural to also have the redshift/rv on the Spectrum* objects and have them be properties that defer to the spectral_axis. But perhaps that means the decision has essentially been made for us that spectra have redshifts, becuase of the need to store the SpectralCoord?

@ibusko
Copy link
Contributor

ibusko commented Aug 21, 2019

@eteq the SpectralCoord defined as in #422 seems to me to be the natural place to keep redshift/rv information for the spectrum. Since all the machinery for conversions and frame projections is in there already.

The drawback I see in this is that it's probably not the most natural way users see the redshift/rv property of a spectrum. I would say that dealing with a spectrum.redshift attribute would sound more natural to the end user, than the spectrum.coordinate.redshift way. But there might be use cases in which that would be the most natural way.

Also, spectrum collections would require perhaps a different approach. There might be collections in which one coordinate object should be used (I am thinking about echelle and similar spectra), while in other types of collections (IFUs), separate coordinate instances would be required.

As for requiring spectra having redshifts, maybe we should allow spectra to have a None SpectrumCoordinate attribute. After all, users that don't care for the redshift/rv properties of a spectrum, wouldn't care either for the coordinate attribute, I suppose.

@camipacifici
Copy link

After a conversation among @astrofrog, @eteq, @nmearl, and @camipacifici:

  • Redshift should be stateful on the Spectrum1D object, like spectrum.redshift'. Any operation that uses the redshift, will be called as do_operation(redshift)' and not as `do_operation(spectrum, redshift).
  • When operation is called, there will be a copy of the Spectrum1D object, but not a copy of the spectral data.
  • In complicated cases, the redshift can be re-set (uncertain redshift, multiple possible sources and redshifts, bad redshifts, etc.).
  • For now, as a temporary measure, the redshift can be put directly on Spectrum1D without using SpectralCoord.

A sample of tester scientist will try this approach and see how they like it. Test cases include: working with a single spectrum or a collection; use available methods for conversions between rest and observer frame; methods that calculate the redshift write directly in the Spectrum1D object.

@nmearl
Copy link
Contributor

nmearl commented Jun 8, 2020

Redshift and radial velocity handling is now done via the SpectralCoordinate object.

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

No branches or pull requests

5 participants