astropy / specutils

An Astropy coordinated package for astronomical spectroscopy. Maintainers: @rosteen @keflavich @eteq
http://specutils.readthedocs.io/en/latest/
169 stars 127 forks source link

Add a warning if continuum-normalized analysis functions get non-dimensionless spectra #547

Open eteq opened 5 years ago

eteq commented 5 years ago

In #538 a continuum-subtraction checker was added to catch the case where a user runs an analysis function that expects a zero-baseline spectrum on a spectrum where continuum has not been subtracted.

In #535 we discussed doing something similar for functions requiring continuum normalization (right now I think that's just equivalent_width), but it occurs to me there's an easier heuristic for continuum-normalization: the unit has to be dimensionless because the continuum and spectrum should have the same units so dividing them out gives dimensionless. So perhaps we should just check the unit and raise a warning if it's not dimensionless? That has essentially zero performance penalty (unlike the continuum-checker...)

cc @camipacifici (since it was your use case that originally led to this idea)

eteq commented 4 years ago

Update from #546 - results in https://github.com/astropy/specutils/issues/546#issuecomment-556523323 .

TL;DR is that it's a small (2x or less) effect for functions that operate on the whole spectrum, but potentially very large (~10x) for very large spectra/cubes where you want to do a small operation on only some of them.

So that's an issue. We may have to pick and choose which the decorators get. Probably needs more discussion before implementing beyond #538

eteq commented 4 years ago

~I've collated the list of potential places for this decorator. Below when I say "local" I mean that the measurement is typically in a small spectral region compared to the whole spectrum, whereas "global" means the function operates on the whole spectrum. Relevant because the "local" ones are the ones where the slow-down could be order-of-magnitude and gets worse with spectrum size (because the continuum-checker is a "global" algorithm), whereas the global ones generally get better with spectrum size (based on #546, of order 2x slowdown)~

...

~So the question is: which, if any, do we want the continuum checker implemented in as a decorator by default? My first instinct is only find_lines_threshold, since that was @camipacifici's original case where there was an issue, and the only definitively global one I found. But I'm interested in other viewpoints - @camipacifici (as the inspiration of the original issue), or @keflavich @dhomeier @crawfordsm (as random potential users), do you have any thoughts here?~

EDIT: wrong issue - see #548

eteq commented 4 years ago

Oops! The above comments were in the wrong issue. Copying them to #548 and striking through the above. - the only relevant one for this issue is equivalent width). We can have the full discussion in #548 and then report back here re: equivalen_width