search

desihub / redrock

Redshift fitting for spectroperfectionism
BSD 3-Clause "New" or "Revised" License
22 stars 14 forks source link

inconsistent legendre wavelength basis #291

Closed sbailey closed 5 months ago

sbailey commented 5 months ago

The conversion of wavelength to reduced wavelength for Legendre polynomials currently happens in 3 different places in the Redrock code:

py/redrock/targets.py:186:     self._legendre = { hs:np.array([scipy.special.legendre(i)( (w-wmin)/(wmax-wmin)*2.) for i in range(nleg)]) for hs, w in dwave.items() }
py/redrock/archetypes.py:161:  legendre = np.array([scipy.special.legendre(i)( (wave-wave_min)/(wave_max-wave_min)*2.-1. ) for i in range(deg_legendre)])
py/redrock/fitz.py:115:        legendre = { hs:np.array([scipy.special.legendre(i)( (w-wmin)/(wmax-wmin)*2.) for i in range(nleg)]) for hs, w in dwave.items() }

The definition in archetypes.py line 161 is correct to transform it to a [-1,1] range:

(wave - wave_min) / (wave_max - wave_min) * 2 - 1

which is the same as

((wave - wave_min) / (wave_max - wave_min)) * 2 - 1

where I added extra parentheses to emphasize that the division happens before the *2 multiplication, i.e. the 2 is multiplied by the numerator, not the denominator.

The other two definitions are missing the "-1" term and thus have a range of [0,2].

Clean this up to be both correct and consistent. I suggest that we add a utility function to do this transformation in a single place, e.g.

def reduced_wavelength(wave):
    wave = np.asarray(wave)
    wavemax = wave.max()
    wavemin = wave.min()
    return 2*(wave - wavemax) / (wavemax - wavemin) - 1
moustakas commented 5 months ago

Done in #299.