search

desihub / redrock

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

fixing inconsistent legendre wavelength basis (#291) #299

Closed abhi0395 closed 2 months ago

abhi0395 commented 2 months ago

Thanks @sbailey for pointing me to the issue of inconsistent legendre wavelength basis definition in code. The issue is detailed in issue#291 have now defined a separate utility function reduced_wavelength() in utils.py, which is called whenever we want to define legendre wavelength basis. It makes the code cleaner and consistent in archetype mode. This consistent definition should also be included in PR#293 and PR#283.

It also fixes a minor case, when an end user does not want to add a prior in the archetype mode. I have implemented a minor logic, that if --archetype-legendre-prior flag is <=0, then no prior will be added in archetype mode.

coveralls commented 2 months ago

Coverage Status

coverage: 38.666% (-0.7%) from 39.333% when pulling cc3c2f4f05ef2b2ef928f00adac142c8bf96308f on correct_legendre into 2ee8544528414aca76f1ca1954d216044ae04cca on main.

sbailey commented 2 months ago

Thanks. For the record, this PR also updates the wavelength -> [-1,1] mapping to be per-camera instead of using the full wavelength basis across all cameras. I think that is what we want so that the legendre polynomials are orthogonal per camera.

I also added unit tests on reduced_wavelength which caught a wavemax vs. wavemin bug which I corrected.

abhi0395 commented 2 months ago

Thank you so much. Luckily, I hadn't run anything big yet.