desihub / redrock

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

add additive broadband parameters #39

Closed ngbusca closed 6 months ago

ngbusca commented 7 years ago

I'm opening this issue to discuss how to implement the additive broadband terms. The easiest seems to be to extend the PCA components in each template with polynomials of lambda_rest. The redrock will handle them completely transparently. I've done tests in branch refactor_io and it works very well.

The caveat to this is that they would get convolved with the resolution, so physically it represents an additive component to the pre-convolved model instead of something that comes in after. In practice, a polynomial is sufficiently smooth so it might not make a difference.

julienguy commented 7 years ago

Convolving any spectrum by the resolution is CPU costly. There must be a way to directly pass the monomials to the fitter.

On Mon, Oct 30, 2017 at 11:37 AM, Nicolas Busca notifications@github.com wrote:

I'm opening this issue to discuss how to implement the additive broadband terms. The easiest seems to be to extend the PCA components in each template with polynomials of lambda_rest. The redrock will handle them completely transparently. I've done tests in branch refactor_io and it works very well.

The caveat to this is that they would get convolved with the resolution, so physically it represents an additive component to the pre-convolved model instead of something that comes in after. In practice, a polynomial is sufficiently smooth so it might not make a difference.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/desihub/redrock/issues/39, or mute the thread https://github.com/notifications/unsubscribe-auth/AE854Lj4e5wuYABnbT7MZ12_vnJMCZmcks5sxhd7gaJpZM4QLqMy .

sbailey commented 7 years ago

If we (re-)add optional broadband terms (additive or multiplicative), they should be in the observer frame per spectrum such that the same functional form is included at any redshift. Including them as extra PCA components is easy, but it means that template comparisons at different redshifts sample different portions of the broadband polynomial functions, and it doesn't allow one camera to be screwed up while others are ok.

An earlier version of redrock did have this, but I took them out as part of a refactor since it does considerably clutter the code and we weren't using them at the time. For comparison, an example snapshot of the state of affairs when it did have optional polynomials:

https://github.com/desihub/redrock/blob/a75d04d04049086f70bda020a1cf09e2fe55de85/py/redrock/zscan.py

In the current code, additive polynomials per spectrum would be included as extra columns of the Tb matrix created in zscan.py lines 197-202. WARNING: that is undercommented code setting up some messy linear algebra, so watch out...

ngbusca commented 7 years ago

'If we (re-)add optional broadband terms (additive or multiplicative), they should be in the observer frame per spectrum such that the same functional form is included at any redshift. Including them as extra PCA components is easy, but it means that template comparisons at different redshifts sample different portions of the broadband polynomial functions, and it doesn't allow one camera to be screwed up while others are ok.'

I don't understand this paragraph. Redshfiting a polynomial is equivalent to redefining its coefficients, isn' it? So the functional form stays the same... maybe there's something obvious I'm missing?

ngbusca commented 7 years ago

Oh, I had coadds in mind. I can see how it wouldn't work accross cameras where one would like to allow for discontinuities in the bb

londumas commented 6 years ago

Fixed in PR https://github.com/desihub/redrock/pull/84.

sbailey commented 4 years ago

post-facto brief update: I tried adding a cubic polynomial to just the QSO templates and although that dramatically improved the low-z QSO efficiency, it had an almost equal negative impact on QSO purity, and it also degraded the LRG efficiency (previously correct LRG redshifts were getting stolen as incorrect high-z QSOs).

i.e. we may still need some polynomial terms, but proceed with caution and evaluate both completeness and purity for all target classes. Better templates trained on DESI SV data (and perhaps using non-PCA methods) may also reduce the need for polynomials.

From an initial look at DESI data it appears the per target polynomials could help absorb limitations of the model eigenvectors, but per-exposure or per-camera inconsistencies could be better solved by lower-level improvements to the hardware, sky subtraction, and flux calibration rather than empirically absorbing differences as template fitting nuisance terms.

moustakas commented 6 months ago

Not really on the critical path for Redrock anymore. Let's open a new ticket if polynomials are needed beyond the work with Legendre polynomials that's already been done.