-
-
Notifications
You must be signed in to change notification settings - Fork 43
find_arc_lines uncertainty handling fix #321
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,8 @@ | |
|
|
||
| from specutils import Spectrum | ||
|
|
||
| from specreduce.utils.utils import measure_noise | ||
|
|
||
| __all__ = ["find_arc_lines", "match_lines_wcs"] | ||
|
|
||
|
|
||
|
|
@@ -29,8 +31,13 @@ def find_arc_lines( | |
|
|
||
| Parameters | ||
| ---------- | ||
| spectrum : The extracted arc spectrum to search for lines. It should be background-subtracted | ||
| and must have an "uncertainty" attribute. | ||
| spectrum | ||
| The extracted arc spectrum to search for lines. It should be background-subtracted. | ||
| The uncertainty can be any of the Astropy uncertainty types | ||
| (`~astropy.nddata.StdDevUncertainty`, `~astropy.nddata.VarianceUncertainty`, or | ||
| `~astropy.nddata.InverseVariance`); it is converted to a standard deviation | ||
| before the line finding. If the spectrum has no uncertainty, a constant per-pixel | ||
| noise is estimated from the data with `~specreduce.utils.utils.measure_noise`. | ||
|
|
||
| fwhm | ||
| Estimated full-width half-maximum of the lines in pixels. | ||
|
|
@@ -55,9 +62,18 @@ def find_arc_lines( | |
| if fwhm.unit != spectrum.spectral_axis.unit: | ||
| raise ValueError("fwhm must have the same units as spectrum.spectral_axis.") | ||
|
|
||
| # The line finding and fitting are always done using standard deviation uncertainties. | ||
| # If the spectrum has no uncertainty, estimate a constant per-pixel noise from the | ||
| # scatter in the data itself. If it has a variance or inverse variance uncertainty, | ||
| # convert it to a standard deviation. Either way, work on a copy so that the input | ||
| # spectrum is left untouched. | ||
| if spectrum.uncertainty is None: | ||
| spectrum = deepcopy(spectrum) | ||
| spectrum.uncertainty = StdDevUncertainty(np.sqrt(np.abs(spectrum.flux.value))) | ||
| noise = measure_noise(spectrum.flux.value) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This ignores flux = spectrum.flux.value
if spectrum.mask is not None:
flux = np.ma.masked_array(flux, mask=spectrum.mask)
noise = measure_noise(flux)This only helps once the masked-input issue in |
||
| spectrum.uncertainty = StdDevUncertainty(np.full(spectrum.flux.shape, noise)) | ||
| elif not isinstance(spectrum.uncertainty, StdDevUncertainty): | ||
| spectrum = deepcopy(spectrum) | ||
| spectrum.uncertainty = spectrum.uncertainty.represent_as(StdDevUncertainty) | ||
|
|
||
| detected_lines = find_lines_threshold(spectrum, noise_factor=noise_factor) | ||
| detected_lines = detected_lines[detected_lines["line_type"] == "emission"] | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,91 @@ | ||||||||||
| import numpy as np | ||||||||||
| from astropy import units as u | ||||||||||
| from astropy.stats import mad_std, sigma_clipped_stats | ||||||||||
|
|
||||||||||
| from specreduce.core import parse_image | ||||||||||
| from specreduce.tracing import Trace, FlatTrace | ||||||||||
| from specreduce.extract import _ap_weight_image, _align_along_trace | ||||||||||
|
|
||||||||||
| __all__ = ['measure_cross_dispersion_profile', '_align_along_trace'] | ||||||||||
| __all__ = ['measure_cross_dispersion_profile', 'measure_noise', '_align_along_trace'] | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def measure_noise( | ||||||||||
| data: np.ndarray | u.Quantity, | ||||||||||
| axis: int = -1, | ||||||||||
| sigma: float = 3.0, | ||||||||||
| maxiters: int | None = 10, | ||||||||||
| ) -> float | np.ndarray | u.Quantity: | ||||||||||
| """ | ||||||||||
| Estimate the per-pixel noise standard deviation of a spectrum from the data itself. | ||||||||||
|
|
||||||||||
| The estimate is the sigma-clipped median absolute deviation of the second difference | ||||||||||
| of the flux along the dispersion axis, ``2 f[i] - f[i-2] - f[i+2]``, scaled to the | ||||||||||
| standard deviation of a single pixel. Differencing removes any smooth continuum or | ||||||||||
| residual background, so the estimate does not depend on the spectrum being | ||||||||||
| background-subtracted, while the iterative sigma clipping removes the pixels | ||||||||||
| dominated by emission or absorption lines before the scatter is measured. | ||||||||||
|
|
||||||||||
| The second difference of white noise has a variance of six times the per-pixel | ||||||||||
| variance, so the clipped ``mad_std`` of the differences is divided by the square | ||||||||||
| root of six. This is the same differencing used by the DER_SNR algorithm | ||||||||||
| (Stoehr et al. 2008), with the plain median replaced by a sigma-clipped robust | ||||||||||
| standard deviation to reduce the bias from dense line lists. | ||||||||||
|
|
||||||||||
| Parameters | ||||||||||
| ---------- | ||||||||||
| data | ||||||||||
| The flux array. Can be 1D or N-dimensional; for a 2D spectral image the noise | ||||||||||
| is estimated separately along ``axis`` for each row (or column). Non-finite | ||||||||||
| values and masked elements of a masked array are ignored. | ||||||||||
| axis | ||||||||||
| The dispersion axis along which the differences are taken. | ||||||||||
| sigma | ||||||||||
| The clipping threshold in units of the robust standard deviation. | ||||||||||
| maxiters | ||||||||||
| The maximum number of clipping iterations, or `None` to iterate until | ||||||||||
| convergence. | ||||||||||
|
|
||||||||||
| Returns | ||||||||||
| ------- | ||||||||||
| float, ndarray, or Quantity | ||||||||||
| The estimated noise standard deviation. A scalar for 1D input, otherwise an | ||||||||||
| array with ``axis`` removed. If ``data`` is a `~astropy.units.Quantity`, the | ||||||||||
| result carries the same unit. | ||||||||||
|
|
||||||||||
| Notes | ||||||||||
| ----- | ||||||||||
| The estimator assumes that the noise is uncorrelated between pixels two apart. | ||||||||||
| For spectra that have been smoothed or resampled onto a finer grid, the | ||||||||||
| differencing suppresses part of the correlated noise and the result is biased | ||||||||||
| low by up to a few tens of percent. | ||||||||||
| """ | ||||||||||
| unit = None | ||||||||||
| if isinstance(data, u.Quantity): | ||||||||||
| unit = data.unit | ||||||||||
| data = data.value | ||||||||||
| if np.ma.isMaskedArray(data): | ||||||||||
| data = data.astype(float).filled(np.nan) | ||||||||||
| values = np.asarray(data, dtype=float) | ||||||||||
|
|
||||||||||
| if values.ndim == 0 or values.shape[axis] < 5: | ||||||||||
| raise ValueError("measure_noise requires at least 5 pixels along the dispersion axis.") | ||||||||||
|
|
||||||||||
| values = np.moveaxis(values, axis, -1) | ||||||||||
| diff2 = 2.0 * values[..., 2:-2] - values[..., :-4] - values[..., 4:] | ||||||||||
|
|
||||||||||
| # Non-finite differences (from NaN or masked pixels) are excluded through an explicit | ||||||||||
| # mask. The placeholder value is never used, but must be finite to keep astropy from | ||||||||||
| # warning about invalid input. | ||||||||||
| invalid = ~np.isfinite(diff2) | ||||||||||
| diff2 = np.where(invalid, 0.0, diff2) | ||||||||||
| _, _, clipped_std = sigma_clipped_stats( | ||||||||||
| diff2, mask=invalid, sigma=sigma, maxiters=maxiters, stdfunc=mad_std, axis=-1 | ||||||||||
| ) | ||||||||||
|
Comment on lines
+81
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's an issue here with NaN or masked input. With
The same data with A per-row loop over the finite values avoids it, something like (untested): rows = diff2.reshape(-1, diff2.shape[-1])
noise = np.full(rows.shape[0], np.nan)
for i, row in enumerate(rows):
row = row[np.isfinite(row)]
if row.size > 0:
noise[i] = sigma_clipped_stats(
row, sigma=sigma, maxiters=maxiters, stdfunc=mad_std
)[2]
noise = noise.reshape(diff2.shape[:-1]) / np.sqrt(6.0)It would also be good for the NaN test to include a few strong lines so it exercises the clipping. This might be worth reporting upstream to astropy as well. |
||||||||||
| noise = clipped_std / np.sqrt(6.0) | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The MAD collapses to 0 whenever more than half of the second differences are identical, and then every positive pixel passes the detection threshold. Some cases I tried through
The raw-count case is a regression compared with the old |
||||||||||
|
|
||||||||||
| if unit is not None: | ||||||||||
| noise = noise * unit | ||||||||||
| return noise | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def measure_cross_dispersion_profile(image, trace=None, crossdisp_axis=0, | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specreduce.utils.utilsisn't indocs/api.rst(onlyspecreduce.utils.synth_datais), so this cross-reference won't resolve. Withnitpicky = Truethat gives a docs warning, andmeasure_noisewon't appear in the API docs at all. Could you add anautomodapientry for it?The module's
__all__also exposes the private_align_along_trace, so you might want:include: measure_noise, measure_cross_dispersion_profile(or similar) to keep that out of the docs.