Skip to content
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

findApertures fooled by negative images from ABBA in longslit stacked data #445

Open
KathleenLabrie opened this issue Nov 1, 2023 · 2 comments
Labels
algorithm Issues relating to algorithms component - gemini gemini_instruments and geminidr severity-critical 💥 Blocker issue urgency-high Fix ASAP urgency-medium Somewhat normal urgency

Comments

@KathleenLabrie
Copy link
Contributor

KathleenLabrie commented Nov 1, 2023

findApertures fails to correctly identify the sources in an aligned and stacked ABBA 2-D spectrum. The negative images appear to be fooling the algorithm.

See screenshot for the interactive window of findApertures.
Screen Shot 2023-10-31 at 15 29 07

To reproduce:

reduce N20170609S0127_stack.fits -r findApertures -p interactive=True

The stack can be found here: https://drive.google.com/drive/folders/14sssXLi93VYdqOF3Mhm9AK1h8_1tnyR0?usp=sharing

[On Oct31, 2023, was using branch feature/niri_wavecal_merge_with_master]

@KathleenLabrie KathleenLabrie added algorithm Issues relating to algorithms severity-critical 💥 Blocker issue urgency-high Fix ASAP urgency-medium Somewhat normal urgency component - gemini gemini_instruments and geminidr labels Nov 1, 2023
@DBerke
Copy link
Contributor

DBerke commented Dec 11, 2023

findApertures() has a parameter threshold, with the description "Threshold for automatic width determination" with a default value of 0.1. It sets the edges of the apertures as a function of the distance between the peak and the nearest adjacent minimum; with such high SNR those minima are likely in the troughs, as insignificant maxima/minima are culled. Increasing threshold decreases the width of the aperture found (a value of 0.5 is the FWHM according to a comment), and 0.1 is fairly conservative, at least for GNIRS data. Increasing it to 0.25, for instance, gives the following aperture (zoomed to the inner region to show detail):
threshold=0 25_master_zoomed
The value of threshold may need some fine-tuning though, depending on the data; for example a value of 0.2 gives the following asymmetric aperture:
threshold=0 2_master_zoomed
More testing would be needed, but it may be useful to have different default values of threshold depending on the instrument and/or observing mode. (I'd be surprised if there was a "one size fits all" value.)

A related bug which have some bearing on this is that currently in master handling of adjacent minima when culling them from the list of extrema returned culls the wrong one. I don't think this is likely a big problem in most cases, but the bug is fixed in commit 1e446aed11763affdeae0711220d707e80255aa8 in xd_support. It's a simple one-line fix, and when reducing this data with that fix temporarily copied in master the second, spurious aperture present with threshold=0.1 is not found.

@DBerke
Copy link
Contributor

DBerke commented Feb 2, 2024

I've pushed a commit to the xd_support branch which should fix this issue. It should be possible to cherry-pick to other branches without problems. The underlying issue is that the code wasn't written for, and doesn't expect, negative portions of the profile. An aperture's width is determined as a fraction of the distance between its peak and the nearest minimum. This change makes it so that maxima/minima below the median are merged/set to the median of the data before determining aperture limits, since they would otherwise overwhelm the natural minima found in the continuum. In testing this causes the aperture limits to be set much more reasonably as the nearest minima are simply down in the continuum near the peak.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
algorithm Issues relating to algorithms component - gemini gemini_instruments and geminidr severity-critical 💥 Blocker issue urgency-high Fix ASAP urgency-medium Somewhat normal urgency
Projects
None yet
Development

No branches or pull requests

2 participants